diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index b0570c33b6..15128235d4 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -557,11 +557,6 @@ OperatorCreate(const char *operatorName, /* Add dependencies for the entry */ address = makeOperatorDependencies(tup, true, isUpdate); - /* Post creation hook for new operator */ - InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0); - - table_close(pg_operator_desc, RowExclusiveLock); - /* * If a commutator and/or negator link is provided, update the other * operator(s) to point at this one, if they don't already have a link. @@ -579,6 +574,11 @@ OperatorCreate(const char *operatorName, if (OidIsValid(commutatorId) || OidIsValid(negatorId)) OperatorUpd(operatorObjectId, commutatorId, negatorId, false); + /* Post creation hook for new operator */ + InvokeObjectPostCreateHook(OperatorRelationId, operatorObjectId, 0); + + table_close(pg_operator_desc, RowExclusiveLock); + return address; } @@ -692,8 +692,28 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete) t->oprcom = InvalidOid; update_commutator = true; } - else if (!isDelete && !OidIsValid(t->oprcom)) + else if (!isDelete && baseId != t->oprcom) { + /* + * When filling in a shell operator the commutator's oprcom field + * will already be set to the baseId so we don't need to make an + * update in that case. + */ + + if (OidIsValid(t->oprcom)) + { + /* + * When commutator's oprcom is already filled in that means + * that it's part of a pair of commutators and by definition + * the existing oprcom must be equivalent to the baseId + * operator. It doesn't make sense to allow the baseId alter + * or create to succeed in this case so raise an error. + */ + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("commutator commutates another operator"))); + } + t->oprcom = baseId; update_commutator = true; } @@ -737,8 +757,28 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete) t->oprnegate = InvalidOid; update_negator = true; } - else if (!isDelete && !OidIsValid(t->oprnegate)) + else if (!isDelete && baseId != t->oprnegate) { + /* + * When filling in a shell operator the negator's oprnegate field + * will already be set to the baseId so we don't need to make an + * update in that case. + */ + + if (OidIsValid(t->oprnegate)) + { + /* + * When negator's oprnegate is already filled in that means + * that it's part of a pair of negators and by definition the + * existing oprnegate must be equivalent to the baseId + * operator. It doesn't make sense to allow the baseId alter + * or create to succeed in this case so raise an error. + */ + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("negator negates another operator"))); + } + t->oprnegate = baseId; update_negator = true; } diff --git a/src/test/regress/expected/create_operator.out b/src/test/regress/expected/create_operator.out index 49070ce90f..600699f407 100644 --- a/src/test/regress/expected/create_operator.out +++ b/src/test/regress/expected/create_operator.out @@ -296,6 +296,52 @@ CREATE OPERATOR ===!!! ( ); 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 FUNCTION create_op_test_fn(boolean, boolean) +RETURNS boolean AS $$ + SELECT NULL::BOOLEAN; +$$ LANGUAGE sql IMMUTABLE; +CREATE OPERATOR === ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + commutator = ===@@@, + negator = ===!!! +); +CREATE OPERATOR ===!!! ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + negator = === +); +CREATE OPERATOR ===@@@ ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + commutator = === +); +-- test trying to use === as a commutator fails +CREATE OPERATOR ===@ ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + commutator = === +); +ERROR: commutator commutates another operator +-- test trying to use === as a negator fails +CREATE OPERATOR ===@ ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + negator = === +); +ERROR: negator negates another operator +-- cleanup +DROP OPERATOR ===@@@(bool, bool); +DROP OPERATOR ===!!!(bool, bool); +DROP OPERATOR ===(bool, bool); +DROP FUNCTION create_op_test_fn; -- invalid: non-lowercase quoted identifiers CREATE OPERATOR === ( diff --git a/src/test/regress/sql/create_operator.sql b/src/test/regress/sql/create_operator.sql index 168cad3814..03e2157a57 100644 --- a/src/test/regress/sql/create_operator.sql +++ b/src/test/regress/sql/create_operator.sql @@ -246,6 +246,51 @@ CREATE OPERATOR ===!!! ( ); ROLLBACK; +-- test that we can't use part of an existing commutator or negator pair as a +-- commutator or negator +CREATE FUNCTION create_op_test_fn(boolean, boolean) +RETURNS boolean AS $$ + SELECT NULL::BOOLEAN; +$$ LANGUAGE sql IMMUTABLE; +CREATE OPERATOR === ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + commutator = ===@@@, + negator = ===!!! +); +CREATE OPERATOR ===!!! ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + negator = === +); +CREATE OPERATOR ===@@@ ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + commutator = === +); +-- test trying to use === as a commutator fails +CREATE OPERATOR ===@ ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + commutator = === +); +-- test trying to use === as a negator fails +CREATE OPERATOR ===@ ( + leftarg = boolean, + rightarg = boolean, + procedure = create_op_test_fn, + negator = === +); +-- cleanup +DROP OPERATOR ===@@@(bool, bool); +DROP OPERATOR ===!!!(bool, bool); +DROP OPERATOR ===(bool, bool); +DROP FUNCTION create_op_test_fn; + -- invalid: non-lowercase quoted identifiers CREATE OPERATOR === (