Extend ALTER OPERATOR to allow setting more optimization attributes.

Allow the COMMUTATOR, NEGATOR, MERGES, and HASHES attributes to be set
by ALTER OPERATOR.  However, we don't allow COMMUTATOR/NEGATOR to be
changed once set, nor allow the MERGES/HASHES flags to be unset once
set.  Changes like that might invalidate plans already made, and
dealing with the consequences seems like more trouble than it's worth.
The main use-case we foresee for this is to allow addition of missed
properties in extension update scripts, such as extending an existing
operator to support hashing.  So only transitions from not-set to set
states seem very useful.

This patch also causes us to reject some incorrect cases that formerly
resulted in inconsistent catalog state, such as trying to set the
commutator of an operator to be some other operator that already has a
(different) commutator.

While at it, move the InvokeObjectPostCreateHook call for CREATE
OPERATOR to not occur until after we've fixed up commutator or negator
links as needed.  The previous ordering could only be justified by
thinking of the OperatorUpd call as a kind of ALTER OPERATOR step;
but we don't call InvokeObjectPostAlterHook therein.  It seems better
to let the hook see the final state of the operator object.

In the documentation, move the discussion of how to establish
commutator pairs from xoper.sgml to the CREATE OPERATOR ref page.

Tommy Pavlicek, reviewed and editorialized a bit by me

Discussion: https://postgr.es/m/CAEhP-W-vGVzf4udhR5M8Bdv88UYnPrhoSkj3ieR3QNrsGQoqdg@mail.gmail.com

4 files changed, 356 insertions, 16 deletions

diff --git a/src/test/regress/expected/alter_operator.out b/src/test/regress/expected/alter_operator.out
index 71bd4842821..4217ba15de2 100644
--- a/src/test/regress/expected/alter_operator.out
+++ b/src/test/regress/expected/alter_operator.out
@@ -25,7 +25,7 @@ ORDER BY 1;
 (3 rows)
 
 --
--- Reset and set params
+-- Test resetting and setting restrict and join attributes.
 --
 ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE);
 ALTER OPERATOR === (boolean, boolean) SET (JOIN = NONE);
@@ -109,18 +109,10 @@ ORDER BY 1;
 --
 -- Test invalid options.
 --
-ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = ====);
-ERROR:  operator attribute "commutator" cannot be changed
-ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = ====);
-ERROR:  operator attribute "negator" cannot be changed
 ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = non_existent_func);
 ERROR:  function non_existent_func(internal, oid, internal, integer) does not exist
 ALTER OPERATOR === (boolean, boolean) SET (JOIN = non_existent_func);
 ERROR:  function non_existent_func(internal, oid, internal, smallint, internal) does not exist
-ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = !==);
-ERROR:  operator attribute "commutator" cannot be changed
-ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = !==);
-ERROR:  operator attribute "negator" cannot be changed
 -- invalid: non-lowercase quoted identifiers
 ALTER OPERATOR & (bit, bit) SET ("Restrict" = _int_contsel, "Join" = _int_contjoinsel);
 ERROR:  operator attribute "Restrict" not recognized
@@ -131,9 +123,145 @@ CREATE USER regress_alter_op_user;
 SET SESSION AUTHORIZATION regress_alter_op_user;
 ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE);
 ERROR:  must be owner of operator ===
--- Clean up
 RESET SESSION AUTHORIZATION;
+--
+-- Test setting commutator, negator, merges, and hashes attributes,
+-- which can only be set if not already set
+--
+CREATE FUNCTION alter_op_test_fn_bool_real(boolean, real)
+RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE;
+CREATE FUNCTION alter_op_test_fn_real_bool(real, boolean)
+RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE;
+-- operator
+CREATE OPERATOR === (
+    LEFTARG = boolean,
+    RIGHTARG = real,
+    PROCEDURE = alter_op_test_fn_bool_real
+);
+-- commutator
+CREATE OPERATOR ==== (
+    LEFTARG = real,
+    RIGHTARG = boolean,
+    PROCEDURE = alter_op_test_fn_real_bool
+);
+-- negator
+CREATE OPERATOR !==== (
+    LEFTARG = boolean,
+    RIGHTARG = real,
+    PROCEDURE = alter_op_test_fn_bool_real
+);
+-- No-op setting already false hashes and merges to false works
+ALTER OPERATOR === (boolean, real) SET (MERGES = false);
+ALTER OPERATOR === (boolean, real) SET (HASHES = false);
+-- Test setting merges and hashes
+ALTER OPERATOR === (boolean, real) SET (MERGES);
+ALTER OPERATOR === (boolean, real) SET (HASHES);
+SELECT oprcanmerge, oprcanhash
+FROM pg_operator WHERE oprname = '==='
+  AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype;
+ oprcanmerge | oprcanhash 
+-------------+------------
+ t           | t
+(1 row)
+
+-- Test setting commutator
+ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====);
+-- Check that oprcom has been set on both the operator and commutator,
+-- that they reference each other, and that the operator used is the existing
+-- one we created and not a new shell operator.
+SELECT op.oprname AS operator_name, com.oprname AS commutator_name,
+  com.oprcode AS commutator_func
+  FROM pg_operator op
+  INNER JOIN pg_operator com ON (op.oid = com.oprcom AND op.oprcom = com.oid)
+  WHERE op.oprname = '==='
+  AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype;
+ operator_name | commutator_name |      commutator_func       
+---------------+-----------------+----------------------------
+ ===           | ====            | alter_op_test_fn_real_bool
+(1 row)
+
+-- Cannot set self as negator
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = ===);
+ERROR:  operator cannot be its own negator
+-- Test setting negator
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====);
+-- Check that oprnegate has been set on both the operator and negator,
+-- that they reference each other, and that the operator used is the existing
+-- one we created and not a new shell operator.
+SELECT op.oprname AS operator_name, neg.oprname AS negator_name,
+  neg.oprcode AS negator_func
+  FROM pg_operator op
+  INNER JOIN pg_operator neg ON (op.oid = neg.oprnegate AND op.oprnegate = neg.oid)
+  WHERE op.oprname = '==='
+  AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype;
+ operator_name | negator_name |        negator_func        
+---------------+--------------+----------------------------
+ ===           | !====        | alter_op_test_fn_bool_real
+(1 row)
+
+-- Test that no-op set succeeds
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====);
+ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====);
+ALTER OPERATOR === (boolean, real) SET (MERGES);
+ALTER OPERATOR === (boolean, real) SET (HASHES);
+-- Check that the final state of the operator is as we expect
+SELECT oprcanmerge, oprcanhash,
+       pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator,
+       pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator
+  FROM pg_operator WHERE oprname = '==='
+  AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype;
+ oprcanmerge | oprcanhash |         commutator          |           negator            
+-------------+------------+-----------------------------+------------------------------
+ t           | t          | operator ====(real,boolean) | operator !====(boolean,real)
+(1 row)
+
+-- Cannot change commutator, negator, merges, and hashes when already set
+CREATE OPERATOR @= (
+    LEFTARG = real,
+    RIGHTARG = boolean,
+    PROCEDURE = alter_op_test_fn_real_bool
+);
+CREATE OPERATOR @!= (
+    LEFTARG = boolean,
+    RIGHTARG = real,
+    PROCEDURE = alter_op_test_fn_bool_real
+);
+ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = @=);
+ERROR:  operator attribute "commutator" cannot be changed if it has already been set
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = @!=);
+ERROR:  operator attribute "negator" cannot be changed if it has already been set
+ALTER OPERATOR === (boolean, real) SET (MERGES = false);
+ERROR:  operator attribute "merges" cannot be changed if it has already been set
+ALTER OPERATOR === (boolean, real) SET (HASHES = false);
+ERROR:  operator attribute "hashes" cannot be changed if it has already been set
+-- Cannot set an operator that already has a commutator as the commutator
+ALTER OPERATOR @=(real, boolean) SET (COMMUTATOR = ===);
+ERROR:  commutator operator === is already the commutator of operator ====
+-- Cannot set an operator that already has a negator as the negator
+ALTER OPERATOR @!=(boolean, real) SET (NEGATOR = ===);
+ERROR:  negator operator === is already the negator of operator !====
+-- Check no changes made
+SELECT oprcanmerge, oprcanhash,
+       pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator,
+       pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator
+  FROM pg_operator WHERE oprname = '==='
+  AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype;
+ oprcanmerge | oprcanhash |         commutator          |           negator            
+-------------+------------+-----------------------------+------------------------------
+ t           | t          | operator ====(real,boolean) | operator !====(boolean,real)
+(1 row)
+
+--
+-- Clean up
+--
 DROP USER regress_alter_op_user;
 DROP OPERATOR === (boolean, boolean);
+DROP OPERATOR === (boolean, real);
+DROP OPERATOR ==== (real, boolean);
+DROP OPERATOR !==== (boolean, real);
+DROP OPERATOR @= (real, boolean);
+DROP OPERATOR @!= (boolean, real);
 DROP FUNCTION customcontsel(internal, oid, internal, integer);
 DROP FUNCTION alter_op_test_fn(boolean, boolean);
+DROP FUNCTION alter_op_test_fn_bool_real(boolean, real);
+DROP FUNCTION alter_op_test_fn_real_bool(real, boolean);
diff --git a/src/test/regress/expected/create_operator.out b/src/test/regress/expected/create_operator.out
index f71b601f2d2..d776d9c18c3 100644
--- a/src/test/regress/expected/create_operator.out
+++ b/src/test/regress/expected/create_operator.out
@@ -260,6 +260,50 @@ CREATE OPERATOR #*# (
 );
 ERROR:  permission denied for type type_op6
 ROLLBACK;
+-- Should fail. An operator cannot be its own negator.
+BEGIN TRANSACTION;
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    negator = ===
+);
+ERROR:  operator cannot be its own negator
+ROLLBACK;
+-- Should fail. An operator cannot be its own negator. Here we check that
+-- this error is detected when replacing a shell operator.
+BEGIN TRANSACTION;
+-- create a shell operator for ===!!! by referencing it as a commutator
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    commutator = ===!!!
+);
+CREATE OPERATOR ===!!! (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4ne,
+    negator = ===!!!
+);
+ERROR:  operator cannot be its own negator
+ROLLBACK;
+-- test that we can't use part of an existing commutator or negator pair
+-- as a commutator or negator
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    commutator = =
+);
+ERROR:  commutator operator = is already the commutator of operator =
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    negator = <>
+);
+ERROR:  negator operator <> is already the negator of operator =
 -- invalid: non-lowercase quoted identifiers
 CREATE OPERATOR ===
 (
diff --git a/src/test/regress/sql/alter_operator.sql b/src/test/regress/sql/alter_operator.sql
index fd403701657..8faecf78301 100644
--- a/src/test/regress/sql/alter_operator.sql
+++ b/src/test/regress/sql/alter_operator.sql
@@ -22,7 +22,7 @@ WHERE classid = 'pg_operator'::regclass AND
 ORDER BY 1;
 
 --
--- Reset and set params
+-- Test resetting and setting restrict and join attributes.
 --
 
 ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE);
@@ -74,12 +74,8 @@ ORDER BY 1;
 --
 -- Test invalid options.
 --
-ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = ====);
-ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = ====);
 ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = non_existent_func);
 ALTER OPERATOR === (boolean, boolean) SET (JOIN = non_existent_func);
-ALTER OPERATOR === (boolean, boolean) SET (COMMUTATOR = !==);
-ALTER OPERATOR === (boolean, boolean) SET (NEGATOR = !==);
 
 -- invalid: non-lowercase quoted identifiers
 ALTER OPERATOR & (bit, bit) SET ("Restrict" = _int_contsel, "Join" = _int_contjoinsel);
@@ -92,9 +88,138 @@ SET SESSION AUTHORIZATION regress_alter_op_user;
 
 ALTER OPERATOR === (boolean, boolean) SET (RESTRICT = NONE);
 
--- Clean up
 RESET SESSION AUTHORIZATION;
+
+--
+-- Test setting commutator, negator, merges, and hashes attributes,
+-- which can only be set if not already set
+--
+
+CREATE FUNCTION alter_op_test_fn_bool_real(boolean, real)
+RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE;
+
+CREATE FUNCTION alter_op_test_fn_real_bool(real, boolean)
+RETURNS boolean AS $$ SELECT NULL::BOOLEAN; $$ LANGUAGE sql IMMUTABLE;
+
+-- operator
+CREATE OPERATOR === (
+    LEFTARG = boolean,
+    RIGHTARG = real,
+    PROCEDURE = alter_op_test_fn_bool_real
+);
+
+-- commutator
+CREATE OPERATOR ==== (
+    LEFTARG = real,
+    RIGHTARG = boolean,
+    PROCEDURE = alter_op_test_fn_real_bool
+);
+
+-- negator
+CREATE OPERATOR !==== (
+    LEFTARG = boolean,
+    RIGHTARG = real,
+    PROCEDURE = alter_op_test_fn_bool_real
+);
+
+-- No-op setting already false hashes and merges to false works
+ALTER OPERATOR === (boolean, real) SET (MERGES = false);
+ALTER OPERATOR === (boolean, real) SET (HASHES = false);
+
+-- Test setting merges and hashes
+ALTER OPERATOR === (boolean, real) SET (MERGES);
+ALTER OPERATOR === (boolean, real) SET (HASHES);
+SELECT oprcanmerge, oprcanhash
+FROM pg_operator WHERE oprname = '==='
+  AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype;
+
+-- Test setting commutator
+ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====);
+
+-- Check that oprcom has been set on both the operator and commutator,
+-- that they reference each other, and that the operator used is the existing
+-- one we created and not a new shell operator.
+SELECT op.oprname AS operator_name, com.oprname AS commutator_name,
+  com.oprcode AS commutator_func
+  FROM pg_operator op
+  INNER JOIN pg_operator com ON (op.oid = com.oprcom AND op.oprcom = com.oid)
+  WHERE op.oprname = '==='
+  AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype;
+
+-- Cannot set self as negator
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = ===);
+
+-- Test setting negator
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====);
+
+-- Check that oprnegate has been set on both the operator and negator,
+-- that they reference each other, and that the operator used is the existing
+-- one we created and not a new shell operator.
+SELECT op.oprname AS operator_name, neg.oprname AS negator_name,
+  neg.oprcode AS negator_func
+  FROM pg_operator op
+  INNER JOIN pg_operator neg ON (op.oid = neg.oprnegate AND op.oprnegate = neg.oid)
+  WHERE op.oprname = '==='
+  AND op.oprleft = 'boolean'::regtype AND op.oprright = 'real'::regtype;
+
+-- Test that no-op set succeeds
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = !====);
+ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = ====);
+ALTER OPERATOR === (boolean, real) SET (MERGES);
+ALTER OPERATOR === (boolean, real) SET (HASHES);
+
+-- Check that the final state of the operator is as we expect
+SELECT oprcanmerge, oprcanhash,
+       pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator,
+       pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator
+  FROM pg_operator WHERE oprname = '==='
+  AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype;
+
+-- Cannot change commutator, negator, merges, and hashes when already set
+
+CREATE OPERATOR @= (
+    LEFTARG = real,
+    RIGHTARG = boolean,
+    PROCEDURE = alter_op_test_fn_real_bool
+);
+CREATE OPERATOR @!= (
+    LEFTARG = boolean,
+    RIGHTARG = real,
+    PROCEDURE = alter_op_test_fn_bool_real
+);
+
+ALTER OPERATOR === (boolean, real) SET (COMMUTATOR = @=);
+ALTER OPERATOR === (boolean, real) SET (NEGATOR = @!=);
+ALTER OPERATOR === (boolean, real) SET (MERGES = false);
+ALTER OPERATOR === (boolean, real) SET (HASHES = false);
+
+-- Cannot set an operator that already has a commutator as the commutator
+ALTER OPERATOR @=(real, boolean) SET (COMMUTATOR = ===);
+
+-- Cannot set an operator that already has a negator as the negator
+ALTER OPERATOR @!=(boolean, real) SET (NEGATOR = ===);
+
+-- Check no changes made
+SELECT oprcanmerge, oprcanhash,
+       pg_describe_object('pg_operator'::regclass, oprcom, 0) AS commutator,
+       pg_describe_object('pg_operator'::regclass, oprnegate, 0) AS negator
+  FROM pg_operator WHERE oprname = '==='
+  AND oprleft = 'boolean'::regtype AND oprright = 'real'::regtype;
+
+--
+-- Clean up
+--
+
 DROP USER regress_alter_op_user;
+
 DROP OPERATOR === (boolean, boolean);
+DROP OPERATOR === (boolean, real);
+DROP OPERATOR ==== (real, boolean);
+DROP OPERATOR !==== (boolean, real);
+DROP OPERATOR @= (real, boolean);
+DROP OPERATOR @!= (boolean, real);
+
 DROP FUNCTION customcontsel(internal, oid, internal, integer);
 DROP FUNCTION alter_op_test_fn(boolean, boolean);
+DROP FUNCTION alter_op_test_fn_bool_real(boolean, real);
+DROP FUNCTION alter_op_test_fn_real_bool(real, boolean);
diff --git a/src/test/regress/sql/create_operator.sql b/src/test/regress/sql/create_operator.sql
index f53e24db3c4..a3096f17df0 100644
--- a/src/test/regress/sql/create_operator.sql
+++ b/src/test/regress/sql/create_operator.sql
@@ -210,6 +210,49 @@ CREATE OPERATOR #*# (
 );
 ROLLBACK;
 
+-- Should fail. An operator cannot be its own negator.
+BEGIN TRANSACTION;
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    negator = ===
+);
+ROLLBACK;
+
+-- Should fail. An operator cannot be its own negator. Here we check that
+-- this error is detected when replacing a shell operator.
+BEGIN TRANSACTION;
+-- create a shell operator for ===!!! by referencing it as a commutator
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    commutator = ===!!!
+);
+CREATE OPERATOR ===!!! (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4ne,
+    negator = ===!!!
+);
+ROLLBACK;
+
+-- test that we can't use part of an existing commutator or negator pair
+-- as a commutator or negator
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    commutator = =
+);
+CREATE OPERATOR === (
+    leftarg = integer,
+    rightarg = integer,
+    procedure = int4eq,
+    negator = <>
+);
+
 -- invalid: non-lowercase quoted identifiers
 CREATE OPERATOR ===
 (