1 files changed, 172 insertions, 22 deletions

diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index cd7f83136f7..df84b839f53 100644
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -54,6 +54,9 @@
 
 static Oid	ValidateRestrictionEstimator(List *restrictionName);
 static Oid	ValidateJoinEstimator(List *joinName);
+static Oid	ValidateOperatorReference(List *name,
+									  Oid leftTypeId,
+									  Oid rightTypeId);
 
 /*
  * DefineOperator
@@ -360,6 +363,53 @@ ValidateJoinEstimator(List *joinName)
 }
 
 /*
+ * Look up and return the OID of an operator,
+ * given a possibly-qualified name and left and right type IDs.
+ *
+ * Verifies that the operator is defined (not a shell) and owned by
+ * the current user, so that we have permission to associate it with
+ * the operator being altered.  Rejecting shell operators is a policy
+ * choice to help catch mistakes, rather than something essential.
+ */
+static Oid
+ValidateOperatorReference(List *name,
+						  Oid leftTypeId,
+						  Oid rightTypeId)
+{
+	Oid			oid;
+	bool		defined;
+
+	oid = OperatorLookup(name,
+						 leftTypeId,
+						 rightTypeId,
+						 &defined);
+
+	/* These message strings are chosen to match parse_oper.c */
+	if (!OidIsValid(oid))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_FUNCTION),
+				 errmsg("operator does not exist: %s",
+						op_signature_string(name,
+											leftTypeId,
+											rightTypeId))));
+
+	if (!defined)
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_FUNCTION),
+				 errmsg("operator is only a shell: %s",
+						op_signature_string(name,
+											leftTypeId,
+											rightTypeId))));
+
+	if (!object_ownercheck(OperatorRelationId, oid, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_OPERATOR,
+					   NameListToString(name));
+
+	return oid;
+}
+
+
+/*
  * Guts of operator deletion.
  */
 void
@@ -406,6 +456,10 @@ RemoveOperatorById(Oid operOid)
  *		routine implementing ALTER OPERATOR <operator> SET (option = ...).
  *
  * Currently, only RESTRICT and JOIN estimator functions can be changed.
+ * COMMUTATOR, NEGATOR, MERGES, and HASHES attributes can be set if they
+ * have not been set previously.  (Changing or removing one of these
+ * attributes could invalidate existing plans, which seems more trouble
+ * than it's worth.)
  */
 ObjectAddress
 AlterOperator(AlterOperatorStmt *stmt)
@@ -426,6 +480,14 @@ AlterOperator(AlterOperatorStmt *stmt)
 	List	   *joinName = NIL; /* optional join sel. function */
 	bool		updateJoin = false;
 	Oid			joinOid;
+	List	   *commutatorName = NIL;	/* optional commutator operator name */
+	Oid			commutatorOid;
+	List	   *negatorName = NIL;	/* optional negator operator name */
+	Oid			negatorOid;
+	bool		canMerge = false;
+	bool		updateMerges = false;
+	bool		canHash = false;
+	bool		updateHashes = false;
 
 	/* Look up the operator */
 	oprId = LookupOperWithArgs(stmt->opername, false);
@@ -456,6 +518,24 @@ AlterOperator(AlterOperatorStmt *stmt)
 			joinName = param;
 			updateJoin = true;
 		}
+		else if (strcmp(defel->defname, "commutator") == 0)
+		{
+			commutatorName = defGetQualifiedName(defel);
+		}
+		else if (strcmp(defel->defname, "negator") == 0)
+		{
+			negatorName = defGetQualifiedName(defel);
+		}
+		else if (strcmp(defel->defname, "merges") == 0)
+		{
+			canMerge = defGetBoolean(defel);
+			updateMerges = true;
+		}
+		else if (strcmp(defel->defname, "hashes") == 0)
+		{
+			canHash = defGetBoolean(defel);
+			updateHashes = true;
+		}
 
 		/*
 		 * The rest of the options that CREATE accepts cannot be changed.
@@ -464,11 +544,7 @@ AlterOperator(AlterOperatorStmt *stmt)
 		else if (strcmp(defel->defname, "leftarg") == 0 ||
 				 strcmp(defel->defname, "rightarg") == 0 ||
 				 strcmp(defel->defname, "function") == 0 ||
-				 strcmp(defel->defname, "procedure") == 0 ||
-				 strcmp(defel->defname, "commutator") == 0 ||
-				 strcmp(defel->defname, "negator") == 0 ||
-				 strcmp(defel->defname, "hashes") == 0 ||
-				 strcmp(defel->defname, "merges") == 0)
+				 strcmp(defel->defname, "procedure") == 0)
 		{
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -488,7 +564,7 @@ AlterOperator(AlterOperatorStmt *stmt)
 					   NameStr(oprForm->oprname));
 
 	/*
-	 * Look up restriction and join estimators if specified
+	 * Look up OIDs for any parameters specified
 	 */
 	if (restrictionName)
 		restrictionOid = ValidateRestrictionEstimator(restrictionName);
@@ -499,28 +575,79 @@ AlterOperator(AlterOperatorStmt *stmt)
 	else
 		joinOid = InvalidOid;
 
-	/* Perform additional checks, like OperatorCreate does */
-	if (!(OidIsValid(oprForm->oprleft) && OidIsValid(oprForm->oprright)))
+	if (commutatorName)
 	{
-		/* If it's not a binary op, these things mustn't be set: */
-		if (OidIsValid(joinOid))
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-					 errmsg("only binary operators can have join selectivity")));
+		/* commutator has reversed arg types */
+		commutatorOid = ValidateOperatorReference(commutatorName,
+												  oprForm->oprright,
+												  oprForm->oprleft);
+
+		/*
+		 * We don't need to do anything extra for a self commutator as in
+		 * OperatorCreate, since the operator surely exists already.
+		 */
 	}
+	else
+		commutatorOid = InvalidOid;
 
-	if (oprForm->oprresult != BOOLOID)
+	if (negatorName)
 	{
-		if (OidIsValid(restrictionOid))
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-					 errmsg("only boolean operators can have restriction selectivity")));
-		if (OidIsValid(joinOid))
+		negatorOid = ValidateOperatorReference(negatorName,
+											   oprForm->oprleft,
+											   oprForm->oprright);
+
+		/* Must reject self-negation */
+		if (negatorOid == oprForm->oid)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-					 errmsg("only boolean operators can have join selectivity")));
+					 errmsg("operator cannot be its own negator")));
+	}
+	else
+	{
+		negatorOid = InvalidOid;
 	}
 
+	/*
+	 * Check that we're not changing any attributes that might be depended on
+	 * by plans, while allowing no-op updates.
+	 */
+	if (OidIsValid(commutatorOid) && OidIsValid(oprForm->oprcom) &&
+		commutatorOid != oprForm->oprcom)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("operator attribute \"%s\" cannot be changed if it has already been set",
+						"commutator")));
+
+	if (OidIsValid(negatorOid) && OidIsValid(oprForm->oprnegate) &&
+		negatorOid != oprForm->oprnegate)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("operator attribute \"%s\" cannot be changed if it has already been set",
+						"negator")));
+
+	if (updateMerges && oprForm->oprcanmerge && !canMerge)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("operator attribute \"%s\" cannot be changed if it has already been set",
+						"merges")));
+
+	if (updateHashes && oprForm->oprcanhash && !canHash)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("operator attribute \"%s\" cannot be changed if it has already been set",
+						"hashes")));
+
+	/* Perform additional checks, like OperatorCreate does */
+	OperatorValidateParams(oprForm->oprleft,
+						   oprForm->oprright,
+						   oprForm->oprresult,
+						   OidIsValid(commutatorOid),
+						   OidIsValid(negatorOid),
+						   OidIsValid(restrictionOid),
+						   OidIsValid(joinOid),
+						   canMerge,
+						   canHash);
+
 	/* Update the tuple */
 	for (i = 0; i < Natts_pg_operator; ++i)
 	{
@@ -531,12 +658,32 @@ AlterOperator(AlterOperatorStmt *stmt)
 	if (updateRestriction)
 	{
 		replaces[Anum_pg_operator_oprrest - 1] = true;
-		values[Anum_pg_operator_oprrest - 1] = restrictionOid;
+		values[Anum_pg_operator_oprrest - 1] = ObjectIdGetDatum(restrictionOid);
 	}
 	if (updateJoin)
 	{
 		replaces[Anum_pg_operator_oprjoin - 1] = true;
-		values[Anum_pg_operator_oprjoin - 1] = joinOid;
+		values[Anum_pg_operator_oprjoin - 1] = ObjectIdGetDatum(joinOid);
+	}
+	if (OidIsValid(commutatorOid))
+	{
+		replaces[Anum_pg_operator_oprcom - 1] = true;
+		values[Anum_pg_operator_oprcom - 1] = ObjectIdGetDatum(commutatorOid);
+	}
+	if (OidIsValid(negatorOid))
+	{
+		replaces[Anum_pg_operator_oprnegate - 1] = true;
+		values[Anum_pg_operator_oprnegate - 1] = ObjectIdGetDatum(negatorOid);
+	}
+	if (updateMerges)
+	{
+		replaces[Anum_pg_operator_oprcanmerge - 1] = true;
+		values[Anum_pg_operator_oprcanmerge - 1] = BoolGetDatum(canMerge);
+	}
+	if (updateHashes)
+	{
+		replaces[Anum_pg_operator_oprcanhash - 1] = true;
+		values[Anum_pg_operator_oprcanhash - 1] = BoolGetDatum(canHash);
 	}
 
 	tup = heap_modify_tuple(tup, RelationGetDescr(catalog),
@@ -546,6 +693,9 @@ AlterOperator(AlterOperatorStmt *stmt)
 
 	address = makeOperatorDependencies(tup, false, true);
 
+	if (OidIsValid(commutatorOid) || OidIsValid(negatorOid))
+		OperatorUpd(oprId, commutatorOid, negatorOid, false);
+
 	InvokeObjectPostAlterHook(OperatorRelationId, oprId, 0);
 
 	table_close(catalog, NoLock);