Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc94959d

Browse files
committed
Fix DROP OPERATOR to reset oprcom/oprnegate links to the dropped operator.
This avoids leaving dangling links in pg_operator; which while fairlyharmless are also unsightly.While we're at it, simplify OperatorUpd, which went throughheap_modify_tuple for no very good reason considering it had already madea tuple copy it could just scribble on.Roma Sokolov, reviewed by Tomas Vondra, additional hacking by Robert Haasand myself.
1 parentd543170 commitc94959d

File tree

7 files changed

+239
-101
lines changed

7 files changed

+239
-101
lines changed

‎src/backend/catalog/pg_operator.c

Lines changed: 97 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ static Oid OperatorShellMake(const char *operatorName,
5454
OidleftTypeId,
5555
OidrightTypeId);
5656

57-
staticvoidOperatorUpd(OidbaseId,OidcommId,OidnegId);
58-
5957
staticOidget_other_operator(List*otherOp,
6058
OidotherLeftTypeId,OidotherRightTypeId,
6159
constchar*operatorName,OidoperatorNamespace,
@@ -566,7 +564,7 @@ OperatorCreate(const char *operatorName,
566564
commutatorId=operatorObjectId;
567565

568566
if (OidIsValid(commutatorId)||OidIsValid(negatorId))
569-
OperatorUpd(operatorObjectId,commutatorId,negatorId);
567+
OperatorUpd(operatorObjectId,commutatorId,negatorId, false);
570568

571569
returnaddress;
572570
}
@@ -639,125 +637,125 @@ get_other_operator(List *otherOp, Oid otherLeftTypeId, Oid otherRightTypeId,
639637
* OperatorUpd
640638
*
641639
*For a given operator, look up its negator and commutator operators.
642-
*If they are defined, but their negator and commutator fields
643-
*(respectively) are empty, then use the new operator for neg or comm.
644-
*This solves a problem for users who need to insert two new operators
645-
*which are the negator or commutator of each other.
640+
*When isDelete is false, update their negator and commutator fields to
641+
*point back to the given operator; when isDelete is true, update those
642+
*fields to no longer point back to the given operator.
643+
*
644+
*The !isDelete case solves a problem for users who need to insert two new
645+
*operators that are the negator or commutator of each other, while the
646+
*isDelete case is needed so as not to leave dangling OID links behind
647+
*after dropping an operator.
646648
*/
647-
staticvoid
648-
OperatorUpd(OidbaseId,OidcommId,OidnegId)
649+
void
650+
OperatorUpd(OidbaseId,OidcommId,OidnegId,boolisDelete)
649651
{
650-
inti;
651652
Relationpg_operator_desc;
652653
HeapTupletup;
653-
boolnulls[Natts_pg_operator];
654-
boolreplaces[Natts_pg_operator];
655-
Datumvalues[Natts_pg_operator];
656-
657-
for (i=0;i<Natts_pg_operator;++i)
658-
{
659-
values[i]= (Datum)0;
660-
replaces[i]= false;
661-
nulls[i]= false;
662-
}
663654

664655
/*
665-
*check and update the commutator & negator, if necessary
666-
*
667-
*We need a CommandCounterIncrement here in case of a self-commutator
668-
*operator:we'll need to updatethetuple that we just inserted.
656+
*If we're making an operator into its own commutator, then we need a
657+
* command-counter increment here, since we've just inserted the tuple
658+
*we're about to update. But when we're dropping an operator, we can
659+
*skip this becausewe're atthebeginning of the command.
669660
*/
670-
CommandCounterIncrement();
661+
if (!isDelete)
662+
CommandCounterIncrement();
671663

664+
/* Open the relation. */
672665
pg_operator_desc=heap_open(OperatorRelationId,RowExclusiveLock);
673666

674-
tup=SearchSysCacheCopy1(OPEROID,ObjectIdGetDatum(commId));
667+
/* Get a writable copy of the commutator's tuple. */
668+
if (OidIsValid(commId))
669+
tup=SearchSysCacheCopy1(OPEROID,ObjectIdGetDatum(commId));
670+
else
671+
tup=NULL;
675672

676-
/*
677-
* if the commutator and negator are the same operator, do one update. XXX
678-
* this is probably useless code --- I doubt it ever makes sense for
679-
* commutator and negator to be the same thing...
680-
*/
681-
if (commId==negId)
673+
/* Update the commutator's tuple if need be. */
674+
if (HeapTupleIsValid(tup))
682675
{
683-
if (HeapTupleIsValid(tup))
676+
Form_pg_operatort= (Form_pg_operator)GETSTRUCT(tup);
677+
boolupdate_commutator= false;
678+
679+
/*
680+
* Out of due caution, we only change the commutator's oprcom field if
681+
* it has the exact value we expected: InvalidOid when creating an
682+
* operator, or baseId when dropping one.
683+
*/
684+
if (isDelete&&t->oprcom==baseId)
684685
{
685-
Form_pg_operatort= (Form_pg_operator)GETSTRUCT(tup);
686-
687-
if (!OidIsValid(t->oprcom)|| !OidIsValid(t->oprnegate))
688-
{
689-
if (!OidIsValid(t->oprnegate))
690-
{
691-
values[Anum_pg_operator_oprnegate-1]=ObjectIdGetDatum(baseId);
692-
replaces[Anum_pg_operator_oprnegate-1]= true;
693-
}
694-
695-
if (!OidIsValid(t->oprcom))
696-
{
697-
values[Anum_pg_operator_oprcom-1]=ObjectIdGetDatum(baseId);
698-
replaces[Anum_pg_operator_oprcom-1]= true;
699-
}
700-
701-
tup=heap_modify_tuple(tup,
702-
RelationGetDescr(pg_operator_desc),
703-
values,
704-
nulls,
705-
replaces);
706-
707-
simple_heap_update(pg_operator_desc,&tup->t_self,tup);
708-
709-
CatalogUpdateIndexes(pg_operator_desc,tup);
710-
}
686+
t->oprcom=InvalidOid;
687+
update_commutator= true;
688+
}
689+
elseif (!isDelete&& !OidIsValid(t->oprcom))
690+
{
691+
t->oprcom=baseId;
692+
update_commutator= true;
711693
}
712694

713-
heap_close(pg_operator_desc,RowExclusiveLock);
714-
715-
return;
716-
}
717-
718-
/* if commutator and negator are different, do two updates */
719-
720-
if (HeapTupleIsValid(tup)&&
721-
!(OidIsValid(((Form_pg_operator)GETSTRUCT(tup))->oprcom)))
722-
{
723-
values[Anum_pg_operator_oprcom-1]=ObjectIdGetDatum(baseId);
724-
replaces[Anum_pg_operator_oprcom-1]= true;
725-
726-
tup=heap_modify_tuple(tup,
727-
RelationGetDescr(pg_operator_desc),
728-
values,
729-
nulls,
730-
replaces);
731-
732-
simple_heap_update(pg_operator_desc,&tup->t_self,tup);
733-
734-
CatalogUpdateIndexes(pg_operator_desc,tup);
735-
736-
values[Anum_pg_operator_oprcom-1]= (Datum)NULL;
737-
replaces[Anum_pg_operator_oprcom-1]= false;
695+
/* If any columns were found to need modification, update tuple. */
696+
if (update_commutator)
697+
{
698+
simple_heap_update(pg_operator_desc,&tup->t_self,tup);
699+
CatalogUpdateIndexes(pg_operator_desc,tup);
700+
701+
/*
702+
* Do CCI to make the updated tuple visible. We must do this in
703+
* case the commutator is also the negator. (Which would be a
704+
* logic error on the operator definer's part, but that's not a
705+
* good reason to fail here.) We would need a CCI anyway in the
706+
* deletion case for a self-commutator with no negator.
707+
*/
708+
CommandCounterIncrement();
709+
}
738710
}
739711

740-
/* check and update the negator, if necessary */
741-
742-
tup=SearchSysCacheCopy1(OPEROID,ObjectIdGetDatum(negId));
712+
/*
713+
* Similarly find and update the negator, if any.
714+
*/
715+
if (OidIsValid(negId))
716+
tup=SearchSysCacheCopy1(OPEROID,ObjectIdGetDatum(negId));
717+
else
718+
tup=NULL;
743719

744-
if (HeapTupleIsValid(tup)&&
745-
!(OidIsValid(((Form_pg_operator)GETSTRUCT(tup))->oprnegate)))
720+
if (HeapTupleIsValid(tup))
746721
{
747-
values[Anum_pg_operator_oprnegate-1]=ObjectIdGetDatum(baseId);
748-
replaces[Anum_pg_operator_oprnegate-1]= true;
722+
Form_pg_operatort= (Form_pg_operator)GETSTRUCT(tup);
723+
boolupdate_negator= false;
749724

750-
tup=heap_modify_tuple(tup,
751-
RelationGetDescr(pg_operator_desc),
752-
values,
753-
nulls,
754-
replaces);
755-
756-
simple_heap_update(pg_operator_desc,&tup->t_self,tup);
725+
/*
726+
* Out of due caution, we only change the negator's oprnegate field if
727+
* it has the exact value we expected: InvalidOid when creating an
728+
* operator, or baseId when dropping one.
729+
*/
730+
if (isDelete&&t->oprnegate==baseId)
731+
{
732+
t->oprnegate=InvalidOid;
733+
update_negator= true;
734+
}
735+
elseif (!isDelete&& !OidIsValid(t->oprnegate))
736+
{
737+
t->oprnegate=baseId;
738+
update_negator= true;
739+
}
757740

758-
CatalogUpdateIndexes(pg_operator_desc,tup);
741+
/* If any columns were found to need modification, update tuple. */
742+
if (update_negator)
743+
{
744+
simple_heap_update(pg_operator_desc,&tup->t_self,tup);
745+
CatalogUpdateIndexes(pg_operator_desc,tup);
746+
747+
/*
748+
* In the deletion case, do CCI to make the updated tuple visible.
749+
* We must do this in case the operator is its own negator. (Which
750+
* would be a logic error on the operator definer's part, but
751+
* that's not a good reason to fail here.)
752+
*/
753+
if (isDelete)
754+
CommandCounterIncrement();
755+
}
759756
}
760757

758+
/* Close relation and release catalog lock. */
761759
heap_close(pg_operator_desc,RowExclusiveLock);
762760
}
763761

‎src/backend/commands/operatorcmds.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,32 @@ RemoveOperatorById(Oid operOid)
341341
{
342342
Relationrelation;
343343
HeapTupletup;
344+
Form_pg_operatorop;
344345

345346
relation=heap_open(OperatorRelationId,RowExclusiveLock);
346347

347348
tup=SearchSysCache1(OPEROID,ObjectIdGetDatum(operOid));
348349
if (!HeapTupleIsValid(tup))/* should not happen */
349350
elog(ERROR,"cache lookup failed for operator %u",operOid);
351+
op= (Form_pg_operator)GETSTRUCT(tup);
352+
353+
/*
354+
* Reset links from commutator and negator, if any. In case of a
355+
* self-commutator or self-negator, this means we have to re-fetch the
356+
* updated tuple. (We could optimize away updates on the tuple we're
357+
* about to drop, but it doesn't seem worth convoluting the logic for.)
358+
*/
359+
if (OidIsValid(op->oprcom)||OidIsValid(op->oprnegate))
360+
{
361+
OperatorUpd(operOid,op->oprcom,op->oprnegate, true);
362+
if (operOid==op->oprcom||operOid==op->oprnegate)
363+
{
364+
ReleaseSysCache(tup);
365+
tup=SearchSysCache1(OPEROID,ObjectIdGetDatum(operOid));
366+
if (!HeapTupleIsValid(tup))/* should not happen */
367+
elog(ERROR,"cache lookup failed for operator %u",operOid);
368+
}
369+
}
350370

351371
simple_heap_delete(relation,&tup->t_self);
352372

‎src/include/catalog/pg_operator_fn.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,6 @@ extern ObjectAddress OperatorCreate(const char *operatorName,
3131

3232
externObjectAddressmakeOperatorDependencies(HeapTupletuple,boolisUpdate);
3333

34+
externvoidOperatorUpd(OidbaseId,OidcommId,OidnegId,boolisDelete);
35+
3436
#endif/* PG_OPERATOR_FN_H */
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
CREATE OPERATOR === (
2+
PROCEDURE = int8eq,
3+
LEFTARG = bigint,
4+
RIGHTARG = bigint,
5+
COMMUTATOR = ===
6+
);
7+
CREATE OPERATOR !== (
8+
PROCEDURE = int8ne,
9+
LEFTARG = bigint,
10+
RIGHTARG = bigint,
11+
NEGATOR = ===,
12+
COMMUTATOR = !==
13+
);
14+
DROP OPERATOR !==(bigint, bigint);
15+
SELECT ctid, oprcom
16+
FROM pg_catalog.pg_operator fk
17+
WHERE oprcom != 0 AND
18+
NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprcom);
19+
ctid | oprcom
20+
------+--------
21+
(0 rows)
22+
23+
SELECT ctid, oprnegate
24+
FROM pg_catalog.pg_operator fk
25+
WHERE oprnegate != 0 AND
26+
NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprnegate);
27+
ctid | oprnegate
28+
------+-----------
29+
(0 rows)
30+
31+
DROP OPERATOR ===(bigint, bigint);
32+
CREATE OPERATOR <| (
33+
PROCEDURE = int8lt,
34+
LEFTARG = bigint,
35+
RIGHTARG = bigint
36+
);
37+
CREATE OPERATOR |> (
38+
PROCEDURE = int8gt,
39+
LEFTARG = bigint,
40+
RIGHTARG = bigint,
41+
NEGATOR = <|,
42+
COMMUTATOR = <|
43+
);
44+
DROP OPERATOR |>(bigint, bigint);
45+
SELECT ctid, oprcom
46+
FROM pg_catalog.pg_operator fk
47+
WHERE oprcom != 0 AND
48+
NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprcom);
49+
ctid | oprcom
50+
------+--------
51+
(0 rows)
52+
53+
SELECT ctid, oprnegate
54+
FROM pg_catalog.pg_operator fk
55+
WHERE oprnegate != 0 AND
56+
NOT EXISTS(SELECT 1 FROM pg_catalog.pg_operator pk WHERE pk.oid = fk.oprnegate);
57+
ctid | oprnegate
58+
------+-----------
59+
(0 rows)
60+
61+
DROP OPERATOR <|(bigint, bigint);

‎src/test/regress/parallel_schedule

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ test: select_into select_distinct select_distinct_on select_implicit select_havi
8484
# ----------
8585
# Another group of parallel tests
8686
# ----------
87-
test: brin gin gist spgist privileges security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets
87+
test: brin gin gist spgist privileges security_label collate matview lock replica_identity rowsecurity object_address tablesample groupingsets drop_operator
8888

8989
# ----------
9090
# Another group of parallel tests

‎src/test/regress/serial_schedule

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ test: union
8989
test: case
9090
test: join
9191
test: aggregates
92-
test: groupingsets
9392
test: transactions
9493
ignore: random
9594
test: random
@@ -114,6 +113,8 @@ test: replica_identity
114113
test: rowsecurity
115114
test: object_address
116115
test: tablesample
116+
test: groupingsets
117+
test: drop_operator
117118
test: alter_generic
118119
test: alter_operator
119120
test: misc

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp