[PATCH] Optional OR REPLACE in CREATE OPERATOR statement

From 7398a8a14f29a6bbb59117a3e3059231f0b476d9 Mon Sep 17 00:00:00 2001
From: Svetlana Derevyanko <s.derevyanko@postgrespro.ru>
Date: Tue, 14 Jun 2022 08:40:51 +0300
Subject: [PATCH v1] Add optional [OR REPLACE] in CREATE OPERATOR statement.

As in ALTER OPERATOR, only restrict and join params can be modified,
because of the cached query plans.
---
 doc/src/sgml/ref/create_operator.sgml         |  11 +-
 src/backend/catalog/pg_operator.c             | 121 +++++++++++++++---
 src/backend/commands/operatorcmds.c           |   6 +-
 src/backend/parser/gram.y                     |  12 ++
 src/backend/tcop/utility.c                    |   3 +-
 src/include/catalog/pg_operator.h             |   3 +-
 src/include/commands/defrem.h                 |   2 +-
 src/test/regress/expected/create_operator.out |  94 ++++++++++++++
 src/test/regress/sql/create_operator.sql      | 101 +++++++++++++++
 9 files changed, 325 insertions(+), 28 deletions(-)

diff --git a/doc/src/sgml/ref/create_operator.sgml b/doc/src/sgml/ref/create_operator.sgml
index e27512ff39..0e03108876 100644
--- a/doc/src/sgml/ref/create_operator.sgml
+++ b/doc/src/sgml/ref/create_operator.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  <refsynopsisdiv>
 <synopsis>
-CREATE OPERATOR <replaceable>name</replaceable> (
+CREATE [ OR REPLACE ] OPERATOR <replaceable>name</replaceable> (
     {FUNCTION|PROCEDURE} = <replaceable class="parameter">function_name</replaceable>
     [, LEFTARG = <replaceable class="parameter">left_type</replaceable> ] [, RIGHTARG = <replaceable class="parameter">right_type</replaceable> ]
     [, COMMUTATOR = <replaceable class="parameter">com_op</replaceable> ] [, NEGATOR = <replaceable class="parameter">neg_op</replaceable> ]
@@ -36,7 +36,8 @@ CREATE OPERATOR <replaceable>name</replaceable> (
 
   <para>
    <command>CREATE OPERATOR</command> defines a new operator,
-   <replaceable class="parameter">name</replaceable>.  The user who
+   <replaceable class="parameter">name</replaceable>.  <command>CREATE OR REPLACE OPERATOR</command>
+   will either create a new operator, or replace an existing definition.  The user who
    defines an operator becomes its owner.  If a schema name is given
    then the operator is created in the specified schema.  Otherwise it
    is created in the current schema.
@@ -114,6 +115,12 @@ CREATE OPERATOR <replaceable>name</replaceable> (
    as <literal>EXECUTE</literal> privilege on the underlying function.  If a
    commutator or negator operator is specified, you must own these operators.
   </para>
+
+  <para>
+   When <command>CREATE OR REPLACE OPERATOR</command> is used to replace an
+   existing operator, only <replaceable class="parameter">res_proc</replaceable>
+   and <replaceable class="parameter">join_proc</replaceable> can be changed.
+  </para>
  </refsect1>
 
  <refsect1>
diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index 630bf3e56c..eca235f735 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -303,6 +303,7 @@ OperatorShellMake(const char *operatorName,
  *		joinId					X join selectivity procedure ID
  *		canMerge				merge join can be used with this operator
  *		canHash					hash join can be used with this operator
+ *		replace					replace operator if exists
  *
  * The caller should have validated properties and permissions for the
  * objects passed as OID references.  We must handle the commutator and
@@ -334,7 +335,8 @@ OperatorCreate(const char *operatorName,
 			   Oid restrictionId,
 			   Oid joinId,
 			   bool canMerge,
-			   bool canHash)
+			   bool canHash,
+			   bool replace)
 {
 	Relation	pg_operator_desc;
 	HeapTuple	tup;
@@ -415,12 +417,18 @@ OperatorCreate(const char *operatorName,
 								   rightTypeId,
 								   &operatorAlreadyDefined);
 
-	if (operatorAlreadyDefined)
+	if (operatorAlreadyDefined && !replace)
 		ereport(ERROR,
 				(errcode(ERRCODE_DUPLICATE_FUNCTION),
 				 errmsg("operator %s already exists",
 						operatorName)));
 
+	/*
+	 * No such operator yet, so CREATE OR REPLACE is equivalent to CREATE
+	 */
+	if (!OidIsValid(operatorObjectId) && replace)
+		replace = false;
+
 	/*
 	 * At this point, if operatorObjectId is not InvalidOid then we are
 	 * filling in a previously-created shell.  Insist that the user own any
@@ -431,6 +439,59 @@ OperatorCreate(const char *operatorName,
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_OPERATOR,
 					   operatorName);
 
+	/*
+	 * When operator is updated,
+	 * params others than RESTRICT and JOIN should remain the same.
+	 */
+	if (replace)
+	{
+		Form_pg_operator oprForm;
+
+		tup = SearchSysCache4(OPERNAMENSP,
+							  PointerGetDatum(operatorName),
+							  ObjectIdGetDatum(leftTypeId),
+							  ObjectIdGetDatum(rightTypeId),
+							  ObjectIdGetDatum(operatorNamespace));
+		oprForm = (Form_pg_operator) GETSTRUCT(tup);
+
+		if (oprForm->oprcanmerge != canMerge)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("operator attribute \"merges\" cannot be changed")));
+
+		if (oprForm->oprcanhash != canHash)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("operator attribute \"hashes\" cannot be changed")));
+
+		commutatorId = commutatorName ? get_other_operator(commutatorName,
+										  rightTypeId, leftTypeId,
+										  operatorName, operatorNamespace,
+										  leftTypeId, rightTypeId,
+										  true) : InvalidOid;
+		if (oprForm->oprcom != commutatorId)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("operator attribute \"commutator\" cannot be changed")));
+
+		negatorId = negatorName ? get_other_operator(negatorName,
+									   leftTypeId, rightTypeId,
+									   operatorName, operatorNamespace,
+									   leftTypeId, rightTypeId,
+									   false) : InvalidOid;
+		if (oprForm->oprnegate != negatorId)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("operator attribute \"negator\" cannot be changed")));
+
+		if (oprForm->oprcode != procedureId)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("operator attribute \"function\" cannot be changed")));
+
+		ReleaseSysCache(tup);
+	}
+
 	/*
 	 * Set up the other operators.  If they do not currently exist, create
 	 * shells in order to get ObjectId's.
@@ -486,25 +547,36 @@ OperatorCreate(const char *operatorName,
 	for (i = 0; i < Natts_pg_operator; ++i)
 	{
 		values[i] = (Datum) NULL;
-		replaces[i] = true;
+		replaces[i] = !replace;
 		nulls[i] = false;
 	}
 
-	namestrcpy(&oname, operatorName);
-	values[Anum_pg_operator_oprname - 1] = NameGetDatum(&oname);
-	values[Anum_pg_operator_oprnamespace - 1] = ObjectIdGetDatum(operatorNamespace);
-	values[Anum_pg_operator_oprowner - 1] = ObjectIdGetDatum(GetUserId());
-	values[Anum_pg_operator_oprkind - 1] = CharGetDatum(leftTypeId ? 'b' : 'l');
-	values[Anum_pg_operator_oprcanmerge - 1] = BoolGetDatum(canMerge);
-	values[Anum_pg_operator_oprcanhash - 1] = BoolGetDatum(canHash);
-	values[Anum_pg_operator_oprleft - 1] = ObjectIdGetDatum(leftTypeId);
-	values[Anum_pg_operator_oprright - 1] = ObjectIdGetDatum(rightTypeId);
-	values[Anum_pg_operator_oprresult - 1] = ObjectIdGetDatum(operResultType);
-	values[Anum_pg_operator_oprcom - 1] = ObjectIdGetDatum(commutatorId);
-	values[Anum_pg_operator_oprnegate - 1] = ObjectIdGetDatum(negatorId);
-	values[Anum_pg_operator_oprcode - 1] = ObjectIdGetDatum(procedureId);
-	values[Anum_pg_operator_oprrest - 1] = ObjectIdGetDatum(restrictionId);
-	values[Anum_pg_operator_oprjoin - 1] = ObjectIdGetDatum(joinId);
+	if (!replace)
+	{
+		namestrcpy(&oname, operatorName);
+		values[Anum_pg_operator_oprname - 1] = NameGetDatum(&oname);
+		values[Anum_pg_operator_oprnamespace - 1] = ObjectIdGetDatum(operatorNamespace);
+		values[Anum_pg_operator_oprowner - 1] = ObjectIdGetDatum(GetUserId());
+		values[Anum_pg_operator_oprkind - 1] = CharGetDatum(leftTypeId ? 'b' : 'l');
+		values[Anum_pg_operator_oprcanmerge - 1] = BoolGetDatum(canMerge);
+		values[Anum_pg_operator_oprcanhash - 1] = BoolGetDatum(canHash);
+		values[Anum_pg_operator_oprleft - 1] = ObjectIdGetDatum(leftTypeId);
+		values[Anum_pg_operator_oprright - 1] = ObjectIdGetDatum(rightTypeId);
+		values[Anum_pg_operator_oprresult - 1] = ObjectIdGetDatum(operResultType);
+		values[Anum_pg_operator_oprcom - 1] = ObjectIdGetDatum(commutatorId);
+		values[Anum_pg_operator_oprnegate - 1] = ObjectIdGetDatum(negatorId);
+		values[Anum_pg_operator_oprcode - 1] = ObjectIdGetDatum(procedureId);
+		values[Anum_pg_operator_oprrest - 1] = ObjectIdGetDatum(restrictionId);
+		values[Anum_pg_operator_oprjoin - 1] = ObjectIdGetDatum(joinId);
+	}
+	else
+	{
+		replaces[Anum_pg_operator_oprrest - 1] = true;
+		values[Anum_pg_operator_oprrest - 1] = ObjectIdGetDatum(restrictionId);
+
+		replaces[Anum_pg_operator_oprrest - 1] = true;
+		values[Anum_pg_operator_oprjoin - 1] = ObjectIdGetDatum(joinId);
+	}
 
 	pg_operator_desc = table_open(OperatorRelationId, RowExclusiveLock);
 
@@ -546,10 +618,17 @@ OperatorCreate(const char *operatorName,
 	}
 
 	/* Add dependencies for the entry */
-	address = makeOperatorDependencies(tup, true, isUpdate);
+	address = makeOperatorDependencies(tup, !replace, isUpdate);
 
-	/* Post creation hook for new operator */
-	InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0);
+	if (replace)
+	{
+		InvokeObjectPostAlterHook(OperatorRelationId, operatorObjectId, 0);
+	}
+	else
+	{
+		/* Post creation hook for new operator */
+		InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0);
+	}
 
 	table_close(pg_operator_desc, RowExclusiveLock);
 
diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index a5924d7d56..474f1e0d8c 100644
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -62,7 +62,7 @@ static Oid	ValidateJoinEstimator(List *joinName);
  * 'parameters' is a list of DefElem
  */
 ObjectAddress
-DefineOperator(List *names, List *parameters)
+DefineOperator(List *names, List *parameters, bool replace)
 {
 	char	   *oprName;
 	Oid			oprNamespace;
@@ -261,7 +261,9 @@ DefineOperator(List *names, List *parameters)
 					   restrictionOid,	/* optional restrict. sel. function */
 					   joinOid, /* optional join sel. function name */
 					   canMerge,	/* operator merges */
-					   canHash);	/* operator hashes */
+					   canHash,	/* operator hashes */
+					   replace);	/* maybe replacement */
+
 }
 
 /*
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 969c9c158f..d5572b39a3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6194,6 +6194,18 @@ DefineStmt:
 					n->definition = $5;
 					$$ = (Node *) n;
 				}
+			| CREATE OR REPLACE OPERATOR any_operator definition
+				{
+					DefineStmt *n = makeNode(DefineStmt);
+
+					n->kind = OBJECT_OPERATOR;
+					n->oldstyle = false;
+					n->replace = true;
+					n->defnames = $5;
+					n->args = NIL;
+					n->definition = $6;
+					$$ = (Node *) n;
+				}
 			| CREATE OPERATOR any_operator definition
 				{
 					DefineStmt *n = makeNode(DefineStmt);
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 6a5bcded55..6591d14529 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1412,7 +1412,8 @@ ProcessUtilitySlow(ParseState *pstate,
 						case OBJECT_OPERATOR:
 							Assert(stmt->args == NIL);
 							address = DefineOperator(stmt->defnames,
-													 stmt->definition);
+													 stmt->definition,
+													 stmt->replace);
 							break;
 						case OBJECT_TYPE:
 							Assert(stmt->args == NIL);
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index 51263f550f..ebc1772646 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -96,7 +96,8 @@ extern ObjectAddress OperatorCreate(const char *operatorName,
 									Oid restrictionId,
 									Oid joinId,
 									bool canMerge,
-									bool canHash);
+									bool canHash,
+									bool replace);
 
 extern ObjectAddress makeOperatorDependencies(HeapTuple tuple,
 											  bool makeExtensionDep,
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 56d2bb6616..5143f47030 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -75,7 +75,7 @@ extern void interpret_function_parameter_list(ParseState *pstate,
 											  Oid *requiredResultType);
 
 /* commands/operatorcmds.c */
-extern ObjectAddress DefineOperator(List *names, List *parameters);
+extern ObjectAddress DefineOperator(List *names, List *parameters, bool replace);
 extern void RemoveOperatorById(Oid operOid);
 extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt);
 
diff --git a/src/test/regress/expected/create_operator.out b/src/test/regress/expected/create_operator.out
index f71b601f2d..b92e1b8ff5 100644
--- a/src/test/regress/expected/create_operator.out
+++ b/src/test/regress/expected/create_operator.out
@@ -283,3 +283,97 @@ WARNING:  operator attribute "Join" not recognized
 WARNING:  operator attribute "Hashes" not recognized
 WARNING:  operator attribute "Merges" not recognized
 ERROR:  operator function must be specified
+--
+-- CREATE OR REPLACE OPERATOR
+--
+CREATE OR REPLACE FUNCTION fn_op7(int, int)
+RETURNS boolean AS $$
+	SELECT $1 = $2;
+$$ LANGUAGE sql IMMUTABLE;
+CREATE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7
+);
+-- Should work. Duplicate operator definition.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7
+);
+ROLLBACK;
+-- Should work. Change restriction selectivity estimator function.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	RESTRICT = eqsel
+);
+ROLLBACK;
+-- Should work. Change join selectivity estimator function.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	JOIN = eqjoinsel
+);
+ROLLBACK;
+-- Should fail. Underlying function can not be changed.
+BEGIN TRANSACTION;
+CREATE FUNCTION fn_op8(int, int)
+RETURNS boolean AS $$
+	SELECT $1 = $2;
+$$ LANGUAGE sql IMMUTABLE;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op8
+);
+ERROR:  operator attribute "function" cannot be changed
+ROLLBACK;
+-- Should fail. Commutator can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	COMMUTATOR = ===
+);
+ERROR:  operator attribute "commutator" cannot be changed
+ROLLBACK;
+-- Should fail. Negator can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	NEGATOR = !===
+);
+ERROR:  operator attribute "negator" cannot be changed
+ROLLBACK;
+-- Should fail. Hash join support param can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	HASHES
+);
+ERROR:  operator attribute "hashes" cannot be changed
+ROLLBACK;
+-- Should fail. Merge join support param can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	MERGES
+);
+ERROR:  operator attribute "merges" cannot be changed
+ROLLBACK;
+-- Clearing
+DROP OPERATOR === (int, int);
+DROP FUNCTION fn_op7;
diff --git a/src/test/regress/sql/create_operator.sql b/src/test/regress/sql/create_operator.sql
index f53e24db3c..ec6e1484e2 100644
--- a/src/test/regress/sql/create_operator.sql
+++ b/src/test/regress/sql/create_operator.sql
@@ -223,3 +223,104 @@ CREATE OPERATOR ===
 	"Hashes",
 	"Merges"
 );
+
+--
+-- CREATE OR REPLACE OPERATOR
+--
+
+CREATE OR REPLACE FUNCTION fn_op7(int, int)
+RETURNS boolean AS $$
+	SELECT $1 = $2;
+$$ LANGUAGE sql IMMUTABLE;
+
+CREATE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7
+);
+
+-- Should work. Duplicate operator definition.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7
+);
+ROLLBACK;
+
+-- Should work. Change restriction selectivity estimator function.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	RESTRICT = eqsel
+);
+ROLLBACK;
+
+-- Should work. Change join selectivity estimator function.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	JOIN = eqjoinsel
+);
+ROLLBACK;
+
+-- Should fail. Underlying function can not be changed.
+BEGIN TRANSACTION;
+CREATE FUNCTION fn_op8(int, int)
+RETURNS boolean AS $$
+	SELECT $1 = $2;
+$$ LANGUAGE sql IMMUTABLE;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op8
+);
+ROLLBACK;
+
+-- Should fail. Commutator can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	COMMUTATOR = ===
+);
+ROLLBACK;
+
+-- Should fail. Negator can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	NEGATOR = !===
+);
+ROLLBACK;
+
+-- Should fail. Hash join support param can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	HASHES
+);
+ROLLBACK;
+
+-- Should fail. Merge join support param can not be changed.
+BEGIN TRANSACTION;
+CREATE OR REPLACE OPERATOR === (
+	LEFTARG = int,
+    RIGHTARG = int,
+    FUNCTION = fn_op7,
+	MERGES
+);
+ROLLBACK;
+
+-- Clearing
+DROP OPERATOR === (int, int);
+DROP FUNCTION fn_op7;
-- 
2.30.2