diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c index 3cd1899..ab969a4 100644 --- a/src/backend/catalog/pg_operator.c +++ b/src/backend/catalog/pg_operator.c @@ -54,8 +54,6 @@ static Oid OperatorShellMake(const char *operatorName, Oid leftTypeId, Oid rightTypeId); -static void OperatorUpd(Oid baseId, Oid commId, Oid negId); - static Oid get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId, const char *operatorName, Oid operatorNamespace, @@ -566,7 +564,7 @@ OperatorCreate(const char *operatorName, commutatorId = operatorObjectId; if (OidIsValid(commutatorId) || OidIsValid(negatorId)) - OperatorUpd(operatorObjectId, commutatorId, negatorId); + OperatorUpd(operatorObjectId, commutatorId, negatorId, false); return address; } @@ -643,9 +641,14 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId, * (respectively) are empty, then use the new operator for neg or comm. * This solves a problem for users who need to insert two new operators * which are the negator or commutator of each other. + * + * If we are removing given operator, we'd like to reset links on commutator + * and negator, but only if they are point to given operator (i.e. haven't + * been somehow updated. This check is mostly useless, since we forbid + * overwriting oprcom and oprnegate). */ -static void -OperatorUpd(Oid baseId, Oid commId, Oid negId) +void +OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete) { int i; Relation pg_operator_desc; @@ -653,6 +656,7 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId) bool nulls[Natts_pg_operator]; bool replaces[Natts_pg_operator]; Datum values[Natts_pg_operator]; + Datum target; for (i = 0; i < Natts_pg_operator; ++i) { @@ -667,10 +671,16 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId) * We need a CommandCounterIncrement here in case of a self-commutator * operator: we'll need to update the tuple that we just inserted. */ - CommandCounterIncrement(); + if (!isDelete) + CommandCounterIncrement(); pg_operator_desc = heap_open(OperatorRelationId, RowExclusiveLock); + /* When deleting, reset other operator field to InvalidOid, otherwise, + * set it to point to operator being updated + */ + target = (isDelete ? InvalidOid : ObjectIdGetDatum(baseId)); + tup = SearchSysCacheCopy1(OPEROID, ObjectIdGetDatum(commId)); /* @@ -683,18 +693,27 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId) if (HeapTupleIsValid(tup)) { Form_pg_operator t = (Form_pg_operator) GETSTRUCT(tup); - - if (!OidIsValid(t->oprcom) || !OidIsValid(t->oprnegate)) + /* + * When deleting, check, whether other op field points to operator + * being updated. Otherwise, check, whether field is not set. + */ + bool shouldUpdateNegator = (isDelete ? + t->oprnegate == baseId : + !OidIsValid(t->oprnegate)); + bool shouldUpdateCommutator = (isDelete ? + t->oprcom == baseId : + !OidIsValid(t->oprcom)); + if (shouldUpdateNegator || shouldUpdateCommutator) { - if (!OidIsValid(t->oprnegate)) + if (shouldUpdateNegator) { - values[Anum_pg_operator_oprnegate - 1] = ObjectIdGetDatum(baseId); + values[Anum_pg_operator_oprnegate - 1] = target; replaces[Anum_pg_operator_oprnegate - 1] = true; } - if (!OidIsValid(t->oprcom)) + if (shouldUpdateCommutator) { - values[Anum_pg_operator_oprcom - 1] = ObjectIdGetDatum(baseId); + values[Anum_pg_operator_oprcom - 1] = target; replaces[Anum_pg_operator_oprcom - 1] = true; } @@ -717,45 +736,52 @@ OperatorUpd(Oid baseId, Oid commId, Oid negId) /* if commutator and negator are different, do two updates */ - if (HeapTupleIsValid(tup) && - !(OidIsValid(((Form_pg_operator) GETSTRUCT(tup))->oprcom))) + if (HeapTupleIsValid(tup)) { - values[Anum_pg_operator_oprcom - 1] = ObjectIdGetDatum(baseId); - replaces[Anum_pg_operator_oprcom - 1] = true; + Form_pg_operator t = (Form_pg_operator) GETSTRUCT(tup); + /* As above, check whether other operator should be updated */ + if (isDelete ? t->oprcom == baseId : !OidIsValid(t->oprcom)) + { - tup = heap_modify_tuple(tup, - RelationGetDescr(pg_operator_desc), - values, - nulls, - replaces); + values[Anum_pg_operator_oprcom - 1] = target; + replaces[Anum_pg_operator_oprcom - 1] =true; - simple_heap_update(pg_operator_desc, &tup->t_self, tup); + tup = heap_modify_tuple(tup, + RelationGetDescr(pg_operator_desc), + values, + nulls, + replaces); + + simple_heap_update(pg_operator_desc, &tup->t_self, tup); - CatalogUpdateIndexes(pg_operator_desc, tup); + CatalogUpdateIndexes(pg_operator_desc, tup); - values[Anum_pg_operator_oprcom - 1] = (Datum) NULL; - replaces[Anum_pg_operator_oprcom - 1] = false; + values[Anum_pg_operator_oprcom - 1] = (Datum) NULL; + replaces[Anum_pg_operator_oprcom - 1] = false; + } } /* check and update the negator, if necessary */ - tup = SearchSysCacheCopy1(OPEROID, ObjectIdGetDatum(negId)); - if (HeapTupleIsValid(tup) && - !(OidIsValid(((Form_pg_operator) GETSTRUCT(tup))->oprnegate))) + if (HeapTupleIsValid(tup)) { - values[Anum_pg_operator_oprnegate - 1] = ObjectIdGetDatum(baseId); - replaces[Anum_pg_operator_oprnegate - 1] = true; + Form_pg_operator t = (Form_pg_operator) GETSTRUCT(tup); + if (isDelete ? t->oprnegate == baseId : !OidIsValid(t->oprnegate)) + { + values[Anum_pg_operator_oprnegate - 1] = target; + replaces[Anum_pg_operator_oprnegate - 1] = true; - tup = heap_modify_tuple(tup, - RelationGetDescr(pg_operator_desc), - values, - nulls, - replaces); + tup = heap_modify_tuple(tup, + RelationGetDescr(pg_operator_desc), + values, + nulls, + replaces); - simple_heap_update(pg_operator_desc, &tup->t_self, tup); + simple_heap_update(pg_operator_desc, &tup->t_self, tup); - CatalogUpdateIndexes(pg_operator_desc, tup); + CatalogUpdateIndexes(pg_operator_desc, tup); + } } heap_close(pg_operator_desc, RowExclusiveLock); diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c index 664e5d7..b389c24 100644 --- a/src/backend/commands/operatorcmds.c +++ b/src/backend/commands/operatorcmds.c @@ -339,15 +339,30 @@ ValidateJoinEstimator(List *joinName) void RemoveOperatorById(Oid operOid) { - Relation relation; - HeapTuple tup; + Relation relation; + HeapTuple tup; + Form_pg_operator op; relation = heap_open(OperatorRelationId, RowExclusiveLock); tup = SearchSysCache1(OPEROID, ObjectIdGetDatum(operOid)); + op = (Form_pg_operator) GETSTRUCT(tup); if (!HeapTupleIsValid(tup)) /* should not happen */ elog(ERROR, "cache lookup failed for operator %u", operOid); + /* + * Reset links on commutator and negator. Only do that if either + * oprcom or oprnegate is set and given operator is not self-commutator. + * For self-commutator with negator prevent meaningful updates of the + * same tuple by sending InvalidOid. + */ + if (OidIsValid(op->oprnegate) || + (OidIsValid(op->oprcom) && operOid != op->oprcom)) + OperatorUpd(operOid, + operOid == op->oprcom ? InvalidOid : op->oprcom, + op->oprnegate, + true); + simple_heap_delete(relation, &tup->t_self); ReleaseSysCache(tup); diff --git a/src/include/catalog/pg_operator_fn.h b/src/include/catalog/pg_operator_fn.h index 5315e19..3e1c7ae 100644 --- a/src/include/catalog/pg_operator_fn.h +++ b/src/include/catalog/pg_operator_fn.h @@ -30,5 +30,5 @@ extern ObjectAddress OperatorCreate(const char *operatorName, bool canHash); extern ObjectAddress makeOperatorDependencies(HeapTuple tuple, bool isUpdate); - +extern void OperatorUpd(Oid baseId, Oid commId, Oid negId, bool isDelete); #endif /* PG_OPERATOR_FN_H */ diff --git a/src/test/regress/expected/drop_operator.out b/src/test/regress/expected/drop_operator.out new file mode 100644 index 0000000..b6d322f --- /dev/null +++ b/src/test/regress/expected/drop_operator.out @@ -0,0 +1,61 @@ +CREATE OPERATOR === ( + PROCEDURE = int8eq, + LEFTARG = bigint, + RIGHTARG = bigint, + COMMUTATOR = === +); +CREATE OPERATOR !== ( + PROCEDURE = int8ne, + LEFTARG = bigint, + RIGHTARG = bigint, + NEGATOR = ===, + COMMUTATOR = !== +); +DROP OPERATOR !==(bigint, bigint); +SELECT ctid, oprcom +FROM pg_catalog.pg_operator fk +WHERE oprcom != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprcom); + ctid | oprcom +------+-------- +(0 rows) + +SELECT ctid, oprnegate +FROM pg_catalog.pg_operator fk +WHERE oprnegate != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprnegate); + ctid | oprnegate +------+----------- +(0 rows) + +DROP OPERATOR ===(bigint, bigint); +CREATE OPERATOR === ( + PROCEDURE = int8eq, + LEFTARG = bigint, + RIGHTARG = bigint +); +CREATE OPERATOR !== ( + PROCEDURE = int8ne, + LEFTARG = bigint, + RIGHTARG = bigint, + NEGATOR = ===, + COMMUTATOR = === +); +DROP OPERATOR !==(bigint, bigint); +SELECT ctid, oprcom +FROM pg_catalog.pg_operator fk +WHERE oprcom != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprcom); + ctid | oprcom +------+-------- +(0 rows) + +SELECT ctid, oprnegate +FROM pg_catalog.pg_operator fk +WHERE oprnegate != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprnegate); + ctid | oprnegate +------+----------- +(0 rows) + +DROP OPERATOR ===(bigint, bigint); diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index bec0316..731677f 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -79,7 +79,7 @@ ignore: random # ---------- # Another group of parallel tests # ---------- -test: select_into select_distinct select_distinct_on select_implicit select_having subselect union case join aggregates transactions random portals arrays btree_index hash_index update namespace prepared_xacts delete +test: select_into select_distinct select_distinct_on select_implicit select_having subselect union case join aggregates transactions random portals arrays btree_index hash_index update namespace prepared_xacts delete drop_operator # ---------- # Another group of parallel tests diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule index 7e9b319..4e2fe83 100644 --- a/src/test/regress/serial_schedule +++ b/src/test/regress/serial_schedule @@ -115,6 +115,7 @@ test: object_address test: tablesample test: alter_generic test: alter_operator +test: drop_operator test: misc test: psql test: async diff --git a/src/test/regress/sql/drop_operator.sql b/src/test/regress/sql/drop_operator.sql new file mode 100644 index 0000000..435a3da --- /dev/null +++ b/src/test/regress/sql/drop_operator.sql @@ -0,0 +1,56 @@ +CREATE OPERATOR === ( + PROCEDURE = int8eq, + LEFTARG = bigint, + RIGHTARG = bigint, + COMMUTATOR = === +); + +CREATE OPERATOR !== ( + PROCEDURE = int8ne, + LEFTARG = bigint, + RIGHTARG = bigint, + NEGATOR = ===, + COMMUTATOR = !== +); + +DROP OPERATOR !==(bigint, bigint); + +SELECT ctid, oprcom +FROM pg_catalog.pg_operator fk +WHERE oprcom != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprcom); + +SELECT ctid, oprnegate +FROM pg_catalog.pg_operator fk +WHERE oprnegate != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprnegate); + +DROP OPERATOR ===(bigint, bigint); + +CREATE OPERATOR === ( + PROCEDURE = int8eq, + LEFTARG = bigint, + RIGHTARG = bigint +); + +CREATE OPERATOR !== ( + PROCEDURE = int8ne, + LEFTARG = bigint, + RIGHTARG = bigint, + NEGATOR = ===, + COMMUTATOR = === +); + +DROP OPERATOR !==(bigint, bigint); + +SELECT ctid, oprcom +FROM pg_catalog.pg_operator fk +WHERE oprcom != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprcom); + +SELECT ctid, oprnegate +FROM pg_catalog.pg_operator fk +WHERE oprnegate != 0 AND + NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprnegate); + +DROP OPERATOR ===(bigint, bigint);