Skip to content

Commit 2f5c9d9

Browse files
alvherrealvherre
committed
Tweak catalog indexing abstraction for upcoming WARM
Split the existing CatalogUpdateIndexes into two different routines, CatalogTupleInsert and CatalogTupleUpdate, which do both the heap insert/update plus the index update. This removes over 300 lines of boilerplate code all over src/backend/catalog/ and src/backend/commands. The resulting code is much more pleasing to the eye. Also, by encapsulating what happens in detail during an UPDATE, this facilitates the upcoming WARM patch, which is going to add a few more lines to the update case making the boilerplate even more boring. The original CatalogUpdateIndexes is removed; there was only one use left, and since it's just three lines, we can as well expand it in place there. We could keep it, but WARM is going to break all the UPDATE out-of-core callsites anyway, so there seems to be no benefit in doing so. Author: Pavan Deolasee Discussion: https://www.postgr.es/m/CABOikdOcFYSZ4vA2gYfs=M2cdXzXX4qGHeEiW3fu9PCfkHLa2A@mail.gmail.com
1 parent e2090d9 commit 2f5c9d9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+256
-594
lines changed

‎src/backend/catalog/aclchk.c‎

Lines changed: 15 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,7 +1252,7 @@ SetDefaultACL(InternalDefaultACL *iacls)
12521252
values[Anum_pg_default_acl_defaclacl - 1] = PointerGetDatum(new_acl);
12531253

12541254
newtuple = heap_form_tuple(RelationGetDescr(rel), values, nulls);
1255-
simple_heap_insert(rel, newtuple);
1255+
CatalogTupleInsert(rel, newtuple);
12561256
}
12571257
else
12581258
{
@@ -1262,12 +1262,9 @@ SetDefaultACL(InternalDefaultACL *iacls)
12621262

12631263
newtuple = heap_modify_tuple(tuple, RelationGetDescr(rel),
12641264
values, nulls, replaces);
1265-
simple_heap_update(rel, &newtuple->t_self, newtuple);
1265+
CatalogTupleUpdate(rel, &newtuple->t_self, newtuple);
12661266
}
12671267

1268-
/* keep the catalog indexes up to date */
1269-
CatalogUpdateIndexes(rel, newtuple);
1270-
12711268
/* these dependencies don't change in an update */
12721269
if (isNew)
12731270
{
@@ -1697,10 +1694,7 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid, const char *relname,
16971694
newtuple = heap_modify_tuple(attr_tuple, RelationGetDescr(attRelation),
16981695
values, nulls, replaces);
16991696

1700-
simple_heap_update(attRelation, &newtuple->t_self, newtuple);
1701-
1702-
/* keep the catalog indexes up to date */
1703-
CatalogUpdateIndexes(attRelation, newtuple);
1697+
CatalogTupleUpdate(attRelation, &newtuple->t_self, newtuple);
17041698

17051699
/* Update initial privileges for extensions */
17061700
recordExtensionInitPriv(relOid, RelationRelationId, attnum,
@@ -1963,10 +1957,7 @@ ExecGrant_Relation(InternalGrant *istmt)
19631957
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation),
19641958
values, nulls, replaces);
19651959

1966-
simple_heap_update(relation, &newtuple->t_self, newtuple);
1967-
1968-
/* keep the catalog indexes up to date */
1969-
CatalogUpdateIndexes(relation, newtuple);
1960+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
19701961

19711962
/* Update initial privileges for extensions */
19721963
recordExtensionInitPriv(relOid, RelationRelationId, 0, new_acl);
@@ -2156,10 +2147,7 @@ ExecGrant_Database(InternalGrant *istmt)
21562147
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
21572148
nulls, replaces);
21582149

2159-
simple_heap_update(relation, &newtuple->t_self, newtuple);
2160-
2161-
/* keep the catalog indexes up to date */
2162-
CatalogUpdateIndexes(relation, newtuple);
2150+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
21632151

21642152
/* Update the shared dependency ACL info */
21652153
updateAclDependencies(DatabaseRelationId, HeapTupleGetOid(tuple), 0,
@@ -2281,10 +2269,7 @@ ExecGrant_Fdw(InternalGrant *istmt)
22812269
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
22822270
nulls, replaces);
22832271

2284-
simple_heap_update(relation, &newtuple->t_self, newtuple);
2285-
2286-
/* keep the catalog indexes up to date */
2287-
CatalogUpdateIndexes(relation, newtuple);
2272+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
22882273

22892274
/* Update initial privileges for extensions */
22902275
recordExtensionInitPriv(fdwid, ForeignDataWrapperRelationId, 0,
@@ -2410,10 +2395,7 @@ ExecGrant_ForeignServer(InternalGrant *istmt)
24102395
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
24112396
nulls, replaces);
24122397

2413-
simple_heap_update(relation, &newtuple->t_self, newtuple);
2414-
2415-
/* keep the catalog indexes up to date */
2416-
CatalogUpdateIndexes(relation, newtuple);
2398+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
24172399

24182400
/* Update initial privileges for extensions */
24192401
recordExtensionInitPriv(srvid, ForeignServerRelationId, 0, new_acl);
@@ -2537,10 +2519,7 @@ ExecGrant_Function(InternalGrant *istmt)
25372519
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
25382520
nulls, replaces);
25392521

2540-
simple_heap_update(relation, &newtuple->t_self, newtuple);
2541-
2542-
/* keep the catalog indexes up to date */
2543-
CatalogUpdateIndexes(relation, newtuple);
2522+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
25442523

25452524
/* Update initial privileges for extensions */
25462525
recordExtensionInitPriv(funcId, ProcedureRelationId, 0, new_acl);
@@ -2671,10 +2650,7 @@ ExecGrant_Language(InternalGrant *istmt)
26712650
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
26722651
nulls, replaces);
26732652

2674-
simple_heap_update(relation, &newtuple->t_self, newtuple);
2675-
2676-
/* keep the catalog indexes up to date */
2677-
CatalogUpdateIndexes(relation, newtuple);
2653+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
26782654

26792655
/* Update initial privileges for extensions */
26802656
recordExtensionInitPriv(langId, LanguageRelationId, 0, new_acl);
@@ -2813,10 +2789,7 @@ ExecGrant_Largeobject(InternalGrant *istmt)
28132789
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation),
28142790
values, nulls, replaces);
28152791

2816-
simple_heap_update(relation, &newtuple->t_self, newtuple);
2817-
2818-
/* keep the catalog indexes up to date */
2819-
CatalogUpdateIndexes(relation, newtuple);
2792+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
28202793

28212794
/* Update initial privileges for extensions */
28222795
recordExtensionInitPriv(loid, LargeObjectRelationId, 0, new_acl);
@@ -2941,10 +2914,7 @@ ExecGrant_Namespace(InternalGrant *istmt)
29412914
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
29422915
nulls, replaces);
29432916

2944-
simple_heap_update(relation, &newtuple->t_self, newtuple);
2945-
2946-
/* keep the catalog indexes up to date */
2947-
CatalogUpdateIndexes(relation, newtuple);
2917+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
29482918

29492919
/* Update initial privileges for extensions */
29502920
recordExtensionInitPriv(nspid, NamespaceRelationId, 0, new_acl);
@@ -3068,10 +3038,7 @@ ExecGrant_Tablespace(InternalGrant *istmt)
30683038
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
30693039
nulls, replaces);
30703040

3071-
simple_heap_update(relation, &newtuple->t_self, newtuple);
3072-
3073-
/* keep the catalog indexes up to date */
3074-
CatalogUpdateIndexes(relation, newtuple);
3041+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
30753042

30763043
/* Update the shared dependency ACL info */
30773044
updateAclDependencies(TableSpaceRelationId, tblId, 0,
@@ -3205,10 +3172,7 @@ ExecGrant_Type(InternalGrant *istmt)
32053172
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
32063173
nulls, replaces);
32073174

3208-
simple_heap_update(relation, &newtuple->t_self, newtuple);
3209-
3210-
/* keep the catalog indexes up to date */
3211-
CatalogUpdateIndexes(relation, newtuple);
3175+
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
32123176

32133177
/* Update initial privileges for extensions */
32143178
recordExtensionInitPriv(typId, TypeRelationId, 0, new_acl);
@@ -5751,10 +5715,7 @@ recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, Acl *new_a
57515715
oldtuple = heap_modify_tuple(oldtuple, RelationGetDescr(relation),
57525716
values, nulls, replace);
57535717

5754-
simple_heap_update(relation, &oldtuple->t_self, oldtuple);
5755-
5756-
/* keep the catalog indexes up to date */
5757-
CatalogUpdateIndexes(relation, oldtuple);
5718+
CatalogTupleUpdate(relation, &oldtuple->t_self, oldtuple);
57585719
}
57595720
else
57605721
/* new_acl is NULL, so delete the entry we found. */
@@ -5788,10 +5749,7 @@ recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid, Acl *new_a
57885749

57895750
tuple = heap_form_tuple(RelationGetDescr(relation), values, nulls);
57905751

5791-
simple_heap_insert(relation, tuple);
5792-
5793-
/* keep the catalog indexes up to date */
5794-
CatalogUpdateIndexes(relation, tuple);
5752+
CatalogTupleInsert(relation, tuple);
57955753
}
57965754
}
57975755

‎src/backend/catalog/heap.c‎

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,13 @@ InsertPgAttributeTuple(Relation pg_attribute_rel,
635635
if (indstate != NULL)
636636
CatalogIndexInsert(indstate, tup);
637637
else
638-
CatalogUpdateIndexes(pg_attribute_rel, tup);
638+
{
639+
CatalogIndexState indstate;
640+
641+
indstate = CatalogOpenIndexes(pg_attribute_rel);
642+
CatalogIndexInsert(indstate, tup);
643+
CatalogCloseIndexes(indstate);
644+
}
639645

640646
heap_freetuple(tup);
641647
}
@@ -824,9 +830,7 @@ InsertPgClassTuple(Relation pg_class_desc,
824830
HeapTupleSetOid(tup, new_rel_oid);
825831

826832
/* finally insert the new tuple, update the indexes, and clean up */
827-
simple_heap_insert(pg_class_desc, tup);
828-
829-
CatalogUpdateIndexes(pg_class_desc, tup);
833+
CatalogTupleInsert(pg_class_desc, tup);
830834

831835
heap_freetuple(tup);
832836
}
@@ -1599,10 +1603,7 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
15991603
"........pg.dropped.%d........", attnum);
16001604
namestrcpy(&(attStruct->attname), newattname);
16011605

1602-
simple_heap_update(attr_rel, &tuple->t_self, tuple);
1603-
1604-
/* keep the system catalog indexes current */
1605-
CatalogUpdateIndexes(attr_rel, tuple);
1606+
CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
16061607
}
16071608

16081609
/*
@@ -1731,10 +1732,7 @@ RemoveAttrDefaultById(Oid attrdefId)
17311732

17321733
((Form_pg_attribute) GETSTRUCT(tuple))->atthasdef = false;
17331734

1734-
simple_heap_update(attr_rel, &tuple->t_self, tuple);
1735-
1736-
/* keep the system catalog indexes current */
1737-
CatalogUpdateIndexes(attr_rel, tuple);
1735+
CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
17381736

17391737
/*
17401738
* Our update of the pg_attribute row will force a relcache rebuild, so
@@ -1932,9 +1930,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
19321930
adrel = heap_open(AttrDefaultRelationId, RowExclusiveLock);
19331931

19341932
tuple = heap_form_tuple(adrel->rd_att, values, nulls);
1935-
attrdefOid = simple_heap_insert(adrel, tuple);
1936-
1937-
CatalogUpdateIndexes(adrel, tuple);
1933+
attrdefOid = CatalogTupleInsert(adrel, tuple);
19381934

19391935
defobject.classId = AttrDefaultRelationId;
19401936
defobject.objectId = attrdefOid;
@@ -1964,9 +1960,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
19641960
if (!attStruct->atthasdef)
19651961
{
19661962
attStruct->atthasdef = true;
1967-
simple_heap_update(attrrel, &atttup->t_self, atttup);
1968-
/* keep catalog indexes current */
1969-
CatalogUpdateIndexes(attrrel, atttup);
1963+
CatalogTupleUpdate(attrrel, &atttup->t_self, atttup);
19701964
}
19711965
heap_close(attrrel, RowExclusiveLock);
19721966
heap_freetuple(atttup);
@@ -2561,8 +2555,7 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
25612555
Assert(is_local);
25622556
con->connoinherit = true;
25632557
}
2564-
simple_heap_update(conDesc, &tup->t_self, tup);
2565-
CatalogUpdateIndexes(conDesc, tup);
2558+
CatalogTupleUpdate(conDesc, &tup->t_self, tup);
25662559
break;
25672560
}
25682561
}
@@ -2602,10 +2595,7 @@ SetRelationNumChecks(Relation rel, int numchecks)
26022595
{
26032596
relStruct->relchecks = numchecks;
26042597

2605-
simple_heap_update(relrel, &reltup->t_self, reltup);
2606-
2607-
/* keep catalog indexes current */
2608-
CatalogUpdateIndexes(relrel, reltup);
2598+
CatalogTupleUpdate(relrel, &reltup->t_self, reltup);
26092599
}
26102600
else
26112601
{
@@ -3145,10 +3135,7 @@ StorePartitionKey(Relation rel,
31453135

31463136
tuple = heap_form_tuple(RelationGetDescr(pg_partitioned_table), values, nulls);
31473137

3148-
simple_heap_insert(pg_partitioned_table, tuple);
3149-
3150-
/* Update the indexes on pg_partitioned_table */
3151-
CatalogUpdateIndexes(pg_partitioned_table, tuple);
3138+
CatalogTupleInsert(pg_partitioned_table, tuple);
31523139
heap_close(pg_partitioned_table, RowExclusiveLock);
31533140

31543141
/* Mark this relation as dependent on a few things as follows */
@@ -3265,8 +3252,7 @@ StorePartitionBound(Relation rel, Relation parent, Node *bound)
32653252
new_val, new_null, new_repl);
32663253
/* Also set the flag */
32673254
((Form_pg_class) GETSTRUCT(newtuple))->relispartition = true;
3268-
simple_heap_update(classRel, &newtuple->t_self, newtuple);
3269-
CatalogUpdateIndexes(classRel, newtuple);
3255+
CatalogTupleUpdate(classRel, &newtuple->t_self, newtuple);
32703256
heap_freetuple(newtuple);
32713257
heap_close(classRel, RowExclusiveLock);
32723258

‎src/backend/catalog/index.c‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -649,10 +649,7 @@ UpdateIndexRelation(Oid indexoid,
649649
/*
650650
* insert the tuple into the pg_index catalog
651651
*/
652-
simple_heap_insert(pg_index, tuple);
653-
654-
/* update the indexes on pg_index */
655-
CatalogUpdateIndexes(pg_index, tuple);
652+
CatalogTupleInsert(pg_index, tuple);
656653

657654
/*
658655
* close the relation and free the tuple
@@ -1324,8 +1321,7 @@ index_constraint_create(Relation heapRelation,
13241321

13251322
if (dirty)
13261323
{
1327-
simple_heap_update(pg_index, &indexTuple->t_self, indexTuple);
1328-
CatalogUpdateIndexes(pg_index, indexTuple);
1324+
CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
13291325

13301326
InvokeObjectPostAlterHookArg(IndexRelationId, indexRelationId, 0,
13311327
InvalidOid, is_internal);
@@ -2103,8 +2099,7 @@ index_build(Relation heapRelation,
21032099
Assert(!indexForm->indcheckxmin);
21042100

21052101
indexForm->indcheckxmin = true;
2106-
simple_heap_update(pg_index, &indexTuple->t_self, indexTuple);
2107-
CatalogUpdateIndexes(pg_index, indexTuple);
2102+
CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
21082103

21092104
heap_freetuple(indexTuple);
21102105
heap_close(pg_index, RowExclusiveLock);
@@ -3448,8 +3443,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
34483443
indexForm->indisvalid = true;
34493444
indexForm->indisready = true;
34503445
indexForm->indislive = true;
3451-
simple_heap_update(pg_index, &indexTuple->t_self, indexTuple);
3452-
CatalogUpdateIndexes(pg_index, indexTuple);
3446+
CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
34533447

34543448
/*
34553449
* Invalidate the relcache for the table, so that after we commit

‎src/backend/catalog/indexing.c‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,19 +146,49 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
146146
}
147147

148148
/*
149-
* CatalogUpdateIndexes - do all the indexing work for a new catalog tuple
149+
* CatalogTupleInsert - do heap and indexing work for a new catalog tuple
150150
*
151-
* This is a convenience routine for the common case where we only need
152-
* to insert or update a single tuple in a system catalog. Avoid using it for
153-
* multiple tuples, since opening the indexes and building the index info
154-
* structures is moderately expensive.
151+
* This is a convenience routine for the common case of inserting a single
152+
* tuple in a system catalog; it inserts a new heap tuple, keeping indexes
153+
* current. Avoid using it for multiple tuples, since opening the indexes and
154+
* building the index info structures is moderately expensive.
155+
*
156+
* The Oid of the inserted tuple is returned.
157+
*/
158+
Oid
159+
CatalogTupleInsert(Relation heapRel, HeapTuple tup)
160+
{
161+
CatalogIndexState indstate;
162+
Oid oid;
163+
164+
indstate = CatalogOpenIndexes(heapRel);
165+
166+
oid = simple_heap_insert(heapRel, tup);
167+
168+
CatalogIndexInsert(indstate, tup);
169+
CatalogCloseIndexes(indstate);
170+
171+
return oid;
172+
}
173+
174+
/*
175+
* CatalogTupleUpdate - do heap and indexing work for updating a catalog tuple
176+
*
177+
* This is a convenience routine for the common case of updating a single
178+
* tuple in a system catalog; it updates one heap tuple (identified by otid)
179+
* with tup, keeping indexes current. Avoid using it for multiple tuples,
180+
* since opening the indexes and building the index info structures is
181+
* moderately expensive.
155182
*/
156183
void
157-
CatalogUpdateIndexes(Relation heapRel, HeapTuple heapTuple)
184+
CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
158185
{
159186
CatalogIndexState indstate;
160187

161188
indstate = CatalogOpenIndexes(heapRel);
162-
CatalogIndexInsert(indstate, heapTuple);
189+
190+
simple_heap_update(heapRel, otid, tup);
191+
192+
CatalogIndexInsert(indstate, tup);
163193
CatalogCloseIndexes(indstate);
164194
}

0 commit comments

Comments
 (0)