Skip to content

Commit 2108d88

Browse files
ayushdshANJU BHARTI
authored andcommitted
Added back the ENR checks for Babelfish temp tables in various functions missed during
cherry-picking SearchSysCacheLocked1 is used when searching in catcache for oids. This function, apart from searching the catcache, also locks the catalog tuple entry for in-place update. But since system catalogs for ENR are maintained locally in-memory, we don't want to do an inplace-update. Hence, for ENRs we will use SearchSysCache1. Also, when doing a cache invalidation, we want to use PrepareInvalidationState instead of PrepareInplaceInvalidationState Task: BABEL-5965 Signed-off-by: Ayush Shah <ayushdsh@amazon.com> cr: https://code.amazon.com/reviews/CR-219295985 (cherry picked from commit 8eec60901bd3b8134ba36281623ef503270c3408)
1 parent 8a35ab3 commit 2108d88

10 files changed

Lines changed: 97 additions & 31 deletions

File tree

src/backend/access/heap/heapam.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include "commands/vacuum.h"
4646
#include "pgstat.h"
4747
#include "port/pg_bitutils.h"
48+
#include "parser/parser.h"
4849
#include "storage/lmgr.h"
4950
#include "storage/predicate.h"
5051
#include "storage/procarray.h"
@@ -6491,7 +6492,7 @@ heap_inplace_update_and_unlock(Relation relation,
64916492
HeapTuple oldtup, HeapTuple tuple,
64926493
Buffer buffer)
64936494
{
6494-
HeapTupleHeader htup = oldtup->t_data;
6495+
HeapTupleHeader htup;
64956496
uint32 oldlen;
64966497
uint32 newlen;
64976498
char *dst;
@@ -6515,6 +6516,7 @@ heap_inplace_update_and_unlock(Relation relation,
65156516
if (ENRUpdateTuple(relation, tuple))
65166517
return;
65176518

6519+
htup = oldtup->t_data;
65186520
Assert(ItemPointerEquals(&oldtup->t_self, &tuple->t_self));
65196521
oldlen = oldtup->t_len - htup->t_hoff;
65206522
newlen = tuple->t_len - tuple->t_data->t_hoff;

src/backend/access/index/genam.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,11 @@ systable_inplace_update_begin(Relation relation,
894894
return;
895895
}
896896

897+
if (scan->enr)
898+
{
899+
break;
900+
}
901+
897902
slot = scan->slot;
898903
Assert(TTS_IS_BUFFERTUPLE(slot));
899904
bslot = (BufferHeapTupleTableSlot *) slot;
@@ -937,8 +942,17 @@ systable_inplace_update_cancel(void *state)
937942
Relation relation = scan->heap_rel;
938943
TupleTableSlot *slot = scan->slot;
939944
BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot;
940-
HeapTuple oldtup = bslot->base.tuple;
941-
Buffer buffer = bslot->buffer;
945+
HeapTuple oldtup;
946+
Buffer buffer;
947+
948+
if (scan->enr)
949+
{
950+
systable_endscan(scan);
951+
return;
952+
}
953+
954+
oldtup = bslot->base.tuple;
955+
buffer = bslot->buffer;
942956

943957
heap_inplace_unlock(relation, oldtup, buffer);
944958
systable_endscan(scan);

src/backend/catalog/aclchk.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1814,7 +1814,7 @@ ExecGrant_Relation(InternalGrant *istmt)
18141814
Oid ownerId;
18151815
HeapTuple tuple;
18161816
ListCell *cell_colprivs;
1817-
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relOid, ENR_TSQL_TEMP));
1817+
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(relOid));
18181818

18191819
if (is_enr)
18201820
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relOid));
@@ -2164,9 +2164,13 @@ ExecGrant_common(InternalGrant *istmt, Oid classid, AclMode default_privs,
21642164
int nnewmembers;
21652165
Oid *oldmembers;
21662166
Oid *newmembers;
2167-
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, objectid, ENR_TSQL_TEMP));
2167+
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(objectid));
21682168

21692169

2170+
/*
2171+
* Exclude objects ENR that are not backed by storage since the logic in
2172+
* SearchSysCacheLocked1 to lock tuples does not apply.
2173+
*/
21702174
if (is_enr)
21712175
tuple = SearchSysCache1(cacheid, ObjectIdGetDatum(objectid));
21722176
else

src/backend/commands/dbcommands.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
#include "parser/parser.h"
5555
#include "pgstat.h"
5656
#include "postmaster/bgwriter.h"
57+
#include "parser/parser.h"
5758
#include "replication/slot.h"
5859
#include "storage/copydir.h"
5960
#include "storage/fd.h"
@@ -1977,7 +1978,7 @@ RenameDatabase(const char *oldname, const char *newname)
19771978
errdetail_busy_db(notherbackends, npreparedxacts)));
19781979

19791980
/* rename */
1980-
is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, db_id, ENR_TSQL_TEMP));
1981+
is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(db_id));
19811982
if (is_enr)
19821983
newtup = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(db_id));
19831984
else
@@ -1987,6 +1988,10 @@ RenameDatabase(const char *oldname, const char *newname)
19871988
otid = newtup->t_self;
19881989
namestrcpy(&(((Form_pg_database) GETSTRUCT(newtup))->datname), newname);
19891990
CatalogTupleUpdate(rel, &otid, newtup);
1991+
/*
1992+
* ENR relations being backend-local are not locked
1993+
* and hence don't need to be unlocked
1994+
*/
19901995
if (!is_enr)
19911996
UnlockTuple(rel, &otid, InplaceUpdateTupleLock);
19921997

src/backend/commands/indexcmds.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4579,7 +4579,7 @@ update_relispartition(Oid relationId, bool newval)
45794579
HeapTuple tup;
45804580
Relation classRel;
45814581
ItemPointerData otid;
4582-
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relationId, ENR_TSQL_TEMP));
4582+
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(relationId));
45834583

45844584
classRel = table_open(RelationRelationId, RowExclusiveLock);
45854585
if (is_enr)
@@ -4592,6 +4592,10 @@ update_relispartition(Oid relationId, bool newval)
45924592
Assert(((Form_pg_class) GETSTRUCT(tup))->relispartition != newval);
45934593
((Form_pg_class) GETSTRUCT(tup))->relispartition = newval;
45944594
CatalogTupleUpdate(classRel, &otid, tup);
4595+
/*
4596+
* ENR relations being backend-local are not locked
4597+
* and hence don't need to be unlocked
4598+
*/
45954599
if (!is_enr)
45964600
UnlockTuple(classRel, &otid, InplaceUpdateTupleLock);
45974601
heap_freetuple(tup);

src/backend/commands/tablecmds.c

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3819,15 +3819,14 @@ SetRelationTableSpace(Relation rel,
38193819
ItemPointerData otid;
38203820
Form_pg_class rd_rel;
38213821
Oid reloid = RelationGetRelid(rel);
3822-
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, reloid, ENR_TSQL_TEMP));
3822+
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(reloid));
38233823

38243824
Assert(CheckRelationTableSpaceMove(rel, newTableSpaceId));
38253825

38263826
/* Get a modifiable copy of the relation's pg_class row. */
38273827
pg_class = table_open(RelationRelationId, RowExclusiveLock);
3828-
38293828
if (is_enr)
3830-
tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
3829+
tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
38313830
else
38323831
tuple = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(reloid));
38333832
if (!HeapTupleIsValid(tuple))
@@ -3841,6 +3840,10 @@ SetRelationTableSpace(Relation rel,
38413840
if (RelFileNumberIsValid(newRelFilenumber))
38423841
rd_rel->relfilenode = newRelFilenumber;
38433842
CatalogTupleUpdate(pg_class, &otid, tuple);
3843+
/*
3844+
* ENR relations being backend-local are not locked
3845+
* and hence don't need to be unlocked
3846+
*/
38443847
if (!is_enr)
38453848
UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);
38463849

@@ -4344,7 +4347,7 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo
43444347
HeapTuple reltup;
43454348
Form_pg_class relform;
43464349
Oid namespaceId;
4347-
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, myrelid, ENR_TSQL_TEMP));
4350+
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(myrelid));
43484351

43494352
/*
43504353
* Grab a lock on the target relation, which we will NOT release until end
@@ -4396,6 +4399,10 @@ RenameRelationInternal(Oid myrelid, const char *newrelname, bool is_internal, bo
43964399
namestrcpy(&(relform->relname), newrelname);
43974400

43984401
CatalogTupleUpdate(relrelation, &otid, reltup);
4402+
/*
4403+
* ENR relations being backend-local are not locked
4404+
* and hence don't need to be unlocked
4405+
*/
43994406
if (!is_enr)
44004407
UnlockTuple(relrelation, &otid, InplaceUpdateTupleLock);
44014408

@@ -6220,7 +6227,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
62206227
BulkInsertState bistate;
62216228
int ti_options;
62226229
ExprState *partqualstate = NULL;
6223-
6230+
62246231
/*
62256232
* Open the relation(s). We have surely already locked the existing
62266233
* table.
@@ -6231,8 +6238,11 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap)
62316238

62326239
if (OidIsValid(OIDNewHeap))
62336240
{
6234-
Assert(CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock,
6235-
false));
6241+
/*
6242+
* With ENRs, we don't hold locks on relation tuples.
6243+
*/
6244+
Assert((sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(OIDNewHeap))
6245+
|| CheckRelationOidLockedByMe(OIDNewHeap, AccessExclusiveLock,false));
62366246
newrel = table_open(OIDNewHeap, NoLock);
62376247
}
62386248
else
@@ -16770,7 +16780,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
1677016780

1677116781
/* Fetch heap tuple */
1677216782
relid = RelationGetRelid(rel);
16773-
is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relid, ENR_TSQL_TEMP));
16783+
is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(relid));
1677416784
if (is_enr)
1677516785
tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
1677616786
else
@@ -19172,13 +19182,13 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
1917219182
Form_pg_class classForm;
1917319183
ObjectAddress thisobj;
1917419184
bool already_done = false;
19175-
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, relOid, ENR_TSQL_TEMP));
19185+
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(relOid));
1917619186

19177-
/* no rel lock for relkind=c so use LOCKTAG_TUPLE */
1917819187
if (is_enr)
1917919188
classTup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relOid));
19180-
else
19189+
else /* no rel lock for relkind=c so use LOCKTAG_TUPLE */
1918119190
classTup = SearchSysCacheLockedCopy1(RELOID, ObjectIdGetDatum(relOid));
19191+
1918219192
if (!HeapTupleIsValid(classTup))
1918319193
elog(ERROR, "cache lookup failed for relation %u", relOid);
1918419194
classForm = (Form_pg_class) GETSTRUCT(classTup);
@@ -19212,22 +19222,30 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
1921219222
classForm->relnamespace = newNspOid;
1921319223

1921419224
CatalogTupleUpdate(classRel, &otid, classTup);
19225+
/*
19226+
* ENR relations being backend-local are not locked
19227+
* and hence don't need to be unlocked
19228+
*/
1921519229
if (!is_enr)
1921619230
UnlockTuple(classRel, &otid, InplaceUpdateTupleLock);
1921719231

19218-
19219-
/* Update dependency on schema if caller said so */
1922019232
if (hasDependEntry &&
1922119233
changeDependencyFor(RelationRelationId,
1922219234
relOid,
1922319235
NamespaceRelationId,
19224-
oldNspOid,
1922519236
newNspOid) != 1)
1922619237
elog(ERROR, "could not change schema dependency for relation \"%s\"",
1922719238
NameStr(classForm->relname));
1922819239
}
19229-
else if (!is_enr)
19230-
UnlockTuple(classRel, &classTup->t_self, InplaceUpdateTupleLock);
19240+
else
19241+
{
19242+
/*
19243+
* ENR relations being backend-local are not locked
19244+
* and hence don't need to be unlocked
19245+
*/
19246+
if (!is_enr)
19247+
UnlockTuple(classRel, &classTup->t_self, InplaceUpdateTupleLock);
19248+
}
1923119249
if (!already_done)
1923219250
{
1923319251
add_exact_object_address(&thisobj, objsMoved);

src/backend/executor/execMain.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,7 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
597597
foreach(l, rangeTable)
598598
{
599599
RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
600+
bool is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(rte->relid));
600601

601602
if (rte->perminfoindex != 0)
602603
{
@@ -610,6 +611,22 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
610611
(rte->rtekind == RTE_SUBQUERY &&
611612
rte->relkind == RELKIND_VIEW));
612613

614+
/*
615+
* Ensure that we have at least an AccessShareLock on relations
616+
* whose permissions need to be checked.
617+
*
618+
* Skip this check in a parallel worker because locks won't be
619+
* taken until ExecInitNode() performs plan initialization.
620+
*
621+
* Skip this check if this relation is in ENR since we don't hold
622+
* locks for relations in ENRs
623+
*
624+
* XXX: ExecCheckPermissions() in a parallel worker may be
625+
* redundant with the checks done in the leader process, so this
626+
* should be reviewed to ensure it’s necessary.
627+
*/
628+
Assert(IsParallelWorker() || is_enr || CheckRelationOidLockedByMe(rte->relid, AccessShareLock, true));
629+
613630
(void) getRTEPermissionInfo(rteperminfos, rte);
614631
/* Many-to-one mapping not allowed */
615632
Assert(!bms_is_member(rte->perminfoindex, indexset));

src/backend/utils/cache/inval.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1756,7 +1756,7 @@ CacheInvalidateENRHeapTuple(Relation relation,
17561756
HeapTuple newtuple)
17571757
{
17581758
CacheInvalidateENRHeapTupleCommon(relation, tuple, newtuple,
1759-
PrepareInplaceInvalidationState);
1759+
PrepareInvalidationState);
17601760
}
17611761

17621762
/*

src/backend/utils/cache/relcache.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3782,7 +3782,7 @@ RelationSetNewRelfilenumber(Relation relation, char persistence)
37823782
MultiXactId minmulti = InvalidMultiXactId;
37833783
TransactionId freezeXid = InvalidTransactionId;
37843784
RelFileLocator newrlocator;
3785-
bool is_enr = false;
3785+
bool is_enr = false;
37863786

37873787
if (!IsBinaryUpgrade)
37883788
{
@@ -3819,14 +3819,13 @@ RelationSetNewRelfilenumber(Relation relation, char persistence)
38193819
* Get a writable copy of the pg_class tuple for the given relation.
38203820
*/
38213821
pg_class = table_open(RelationRelationId, RowExclusiveLock);
3822-
3823-
is_enr = (sql_dialect == SQL_DIALECT_TSQL && get_ENR_withoid(currentQueryEnv, RelationGetRelid(relation), ENR_TSQL_TEMP));
3822+
is_enr = (sql_dialect == SQL_DIALECT_TSQL && GetENRTempTableWithOid(RelationGetRelid(relation)));
38243823
if (is_enr)
38253824
tuple = SearchSysCacheCopy1(RELOID,
3826-
ObjectIdGetDatum(RelationGetRelid(relation)));
3825+
ObjectIdGetDatum(RelationGetRelid(relation)));
38273826
else
38283827
tuple = SearchSysCacheLockedCopy1(RELOID,
3829-
ObjectIdGetDatum(RelationGetRelid(relation)));
3828+
ObjectIdGetDatum(RelationGetRelid(relation)));
38303829
if (!HeapTupleIsValid(tuple))
38313830
elog(ERROR, "could not find tuple for relation %u",
38323831
RelationGetRelid(relation));
@@ -3953,7 +3952,10 @@ RelationSetNewRelfilenumber(Relation relation, char persistence)
39533952

39543953
CatalogTupleUpdate(pg_class, &otid, tuple);
39553954
}
3956-
3955+
/*
3956+
* ENR relations being backend-local are not locked
3957+
* and hence don't need to be unlocked
3958+
*/
39573959
if (!is_enr)
39583960
UnlockTuple(pg_class, &otid, InplaceUpdateTupleLock);
39593961
heap_freetuple(tuple);

src/backend/utils/cache/syscache.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ ReleaseSysCache(HeapTuple tuple)
308308
*
309309
* The returned tuple may be the subject of an uncommitted update, so this
310310
* doesn't prevent the "tuple concurrently updated" error.
311-
*
311+
*
312312
* Note: For Babelfish, this function should not be used if the target tuple is
313313
* for an ENR entry, as there is no physical tid for ENR catalog tuples (since ENR
314314
* entries hold all catalog data internally in their cache). Use SearchSysCache1() instead

0 commit comments

Comments
 (0)