From f7d5b8488bbe2223f215534012c0d30dbe977277 Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Thu, 12 Oct 2023 17:45:59 +0000 Subject: [PATCH 01/13] Support session-local OID buffer for temp tables in TSQL Signed-off-by: Tim Chang add full range of OID Remove ability to alter buffer size GUC, correct error messages Remove lc NULL check Remove references to TempVariableCache Revert "Remove lc NULL check" This reverts commit 07c28e5ac1293548e82f03708c6ae2fea39fe542. Add proper check for toast tables Added comments, added OID macros Add temp table utilization warning Use correct backend for index_create workaround Add include --- src/backend/access/transam/varsup.c | 76 +++++++++ src/backend/catalog/catalog.c | 193 +++++++++++++++++++++- src/backend/catalog/index.c | 12 +- src/backend/catalog/toasting.c | 6 + src/backend/utils/misc/guc.c | 6 + src/backend/utils/misc/queryenvironment.c | 14 +- src/include/access/transam.h | 9 + src/include/catalog/catalog.h | 4 + src/include/utils/guc.h | 3 + src/include/utils/rel.h | 8 + 10 files changed, 320 insertions(+), 11 deletions(-) diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index dd3d7498250..640d4c183ae 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -13,6 +13,8 @@ #include "postgres.h" +#include + #include "access/clog.h" #include "access/commit_ts.h" #include "access/subtrans.h" @@ -24,12 +26,16 @@ #include "postmaster/autovacuum.h" #include "storage/pmsignal.h" #include "storage/proc.h" +#include "utils/guc.h" #include "utils/syscache.h" /* Number of OIDs to prefetch (preallocate) per XLOG write */ #define VAR_OID_PREFETCH 8192 +#define OID_TO_BUFFER_START(oid) ((oid) + INT_MIN) +#define BUFFER_START_TO_OID ((Oid) (temp_oid_buffer_start) - INT_MIN) + /* pointer to "variable cache" in shared memory (set up by shmem.c) */ VariableCache ShmemVariableCache = NULL; @@ -514,6 +520,76 @@ ForceTransactionIdLimitUpdate(void) return false; } +Oid +GetNewTempObjectId(void) +{ + Oid result; + Oid tempOidStart; + static Oid nextTempOid = InvalidOid; + + /* safety check, we should never get this far in a HS standby */ + if (RecoveryInProgress()) + elog(ERROR, "cannot assign OIDs during recovery"); + + if (!OidIsValid((Oid) temp_oid_buffer_start)) /* InvalidOid means it needs assigning */ + { + /* First check to see if another connection has already picked a start, then update. */ + LWLockAcquire(OidGenLock, LW_EXCLUSIVE); + if (OidIsValid(ShmemVariableCache->tempOidStart)) + { + temp_oid_buffer_start = OID_TO_BUFFER_START(ShmemVariableCache->tempOidStart); + nextTempOid = ShmemVariableCache->tempOidStart; + } + else + { + /* We need to pick a new start for the buffer range. */ + tempOidStart = ShmemVariableCache->nextOid; + + /* + * Decrement ShmemVariableCache->oidCount to take into account the new buffer we're allocating + */ + if (ShmemVariableCache->oidCount < temp_oid_buffer_size) + ShmemVariableCache->oidCount = 0; + else + ShmemVariableCache->oidCount -= temp_oid_buffer_size; + + /* + * If ShmemVariableCache->nextOid is below FirstNormalObjectId then we can start at FirstNormalObjectId here and + * GetNewObjectId will return the right value on the next call. + */ + if (tempOidStart < FirstNormalObjectId) + tempOidStart = FirstNormalObjectId; + + /* If the OID range would wraparound, start from beginning instead. */ + if (tempOidStart + temp_oid_buffer_size < tempOidStart) + tempOidStart = FirstNormalObjectId; + + temp_oid_buffer_start = OID_TO_BUFFER_START(tempOidStart); + ShmemVariableCache->tempOidStart = tempOidStart; + ShmemVariableCache->tempOidBufferSize = temp_oid_buffer_size; + + nextTempOid = (Oid) tempOidStart; + + /* Skip nextOid ahead to end of range here as well. */ + ShmemVariableCache->nextOid = (Oid) (tempOidStart + temp_oid_buffer_size); + } + LWLockRelease(OidGenLock); + } + + /* + * Check for wraparound of the temp OID buffer. + */ + if (nextTempOid >= (Oid) (BUFFER_START_TO_OID + temp_oid_buffer_size) + || nextTempOid < BUFFER_START_TO_OID) + { + nextTempOid = BUFFER_START_TO_OID; + } + + result = nextTempOid; + nextTempOid++; + + return result; +} /* * GetNewObjectId -- allocate a new OID diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 168b74fe6ae..385bf25f2c3 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -42,6 +42,7 @@ #include "catalog/pg_tablespace.h" #include "catalog/pg_type.h" #include "miscadmin.h" +#include "parser/parser.h" #include "storage/fd.h" #include "utils/fmgroids.h" #include "utils/fmgrprotos.h" @@ -416,7 +417,7 @@ GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn) /* Only system relations are supported */ Assert(IsSystemRelation(relation)); - /* In bootstrap mode, we don't have any indexes to use */ + /* In bootstrap mode, we don't have any indexes to use. No temp objects in bootstrap mode. */ if (IsBootstrapProcessingMode()) return GetNewObjectId(); @@ -496,6 +497,76 @@ GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn) return newOid; } +Oid +GetNewTempOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn) +{ + Oid newOid; + SysScanDesc scan; + ScanKeyData key; + bool collides; + uint64 retries = 0; + + /* Only system relations are supported */ + Assert(IsSystemRelation(relation)); + + /* In bootstrap mode, we don't have any indexes to use. No temp objects in bootstrap mode. */ + if (IsBootstrapProcessingMode()) + return GetNewObjectId(); + + /* + * We should never be asked to generate a new pg_type OID during + * pg_upgrade; doing so would risk collisions with the OIDs it wants to + * assign. Hitting this assert means there's some path where we failed to + * ensure that a type OID is determined by commands in the dump script. + */ + Assert(!IsBinaryUpgrade || RelationGetRelid(relation) != TypeRelationId); + + /* Generate new OIDs until we find one not in the table */ + do + { + CHECK_FOR_INTERRUPTS(); + + newOid = GetNewTempObjectId(); + + ScanKeyInit(&key, + oidcolumn, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(newOid)); + + /* see notes above about using SnapshotAny */ + scan = systable_beginscan(relation, indexId, true, + SnapshotAny, 1, &key); + + collides = HeapTupleIsValid(systable_getnext(scan)); + + systable_endscan(scan); + + /* + * Log that we iterate more than GETNEWOID_LOG_THRESHOLD but have not + * yet found OID unused in the relation. Then repeat logging with + * exponentially increasing intervals until we iterate more than + * GETNEWOID_LOG_MAX_INTERVAL. Finally repeat logging every + * GETNEWOID_LOG_MAX_INTERVAL unless an unused OID is found. This + * logic is necessary not to fill up the server log with the similar + * messages. + */ + if (OidIsValid((Oid) temp_oid_buffer_start) && retries >= temp_oid_buffer_size) + { + ereport(ERROR, + (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); + } + else if (OidIsValid((Oid) temp_oid_buffer_start) && retries >= (0.8 * temp_oid_buffer_size)) + { + ereport(WARNING, + (errmsg("Temp object OID usage is over 80%%. Consider dropping some temp tables or starting a new session."))); + } + + retries++; + } while (collides); + + return newOid; +} + /* * GetNewRelFileNode * Generate a new relfilenode number that is unique within the @@ -519,6 +590,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) char *rpath; bool collides; BackendId backend; + int tries = 0; /* * If we ever get here during pg_upgrade, there's something wrong; all @@ -558,10 +630,127 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) /* Generate the OID */ if (pg_class) - rnode.node.relNode = GetNewOidWithIndex(pg_class, ClassOidIndexId, + { + if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL) + { + /* Temp tables use temp OID logic */ + rnode.node.relNode = GetNewTempOidWithIndex(pg_class, ClassOidIndexId, + Anum_pg_class_oid); + } + else + { + rnode.node.relNode = GetNewOidWithIndex(pg_class, ClassOidIndexId, Anum_pg_class_oid); + } + } else + { + if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL) + { + /* Temp tables use temp OID logic */ + rnode.node.relNode = GetNewTempObjectId(); + } + else + { + rnode.node.relNode = GetNewObjectId(); + } + } + + /* Check for existing file of same name */ + rpath = relpath(rnode, MAIN_FORKNUM); + + if (access(rpath, F_OK) == 0) + { + /* definite collision */ + collides = true; + } + else + { + /* + * Here we have a little bit of a dilemma: if errno is something + * other than ENOENT, should we declare a collision and loop? In + * practice it seems best to go ahead regardless of the errno. If + * there is a colliding file we will get an smgr failure when we + * attempt to create the new relation file. + */ + collides = false; + } + + if (OidIsValid((Oid) temp_oid_buffer_start) && tries > temp_oid_buffer_size) + { + ereport(ERROR, + (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); + } + else if (OidIsValid((Oid) temp_oid_buffer_start) && tries >= (0.8 * temp_oid_buffer_size)) + { + ereport(WARNING, + (errmsg("Temp object OID usage is over 80%%. Consider dropping some temp tables or starting a new session."))); + } + + pfree(rpath); + tries++; + } while (collides); + + return rnode.node.relNode; +} + +/* + * Until index_create properly uses the temp OID buffer, we need a workaround to provide + * the expected BackendId while not using the temp OID buffer. */ +Oid +GetNewPermanentRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) +{ + RelFileNodeBackend rnode; + char *rpath; + bool collides; + BackendId backend; + + /* + * If we ever get here during pg_upgrade, there's something wrong; all + * relfilenode assignments during a binary-upgrade run should be + * determined by commands in the dump script. + */ + Assert(!IsBinaryUpgrade); + + switch (relpersistence) + { + case RELPERSISTENCE_TEMP: + backend = BackendIdForTempRelations(); + break; + case RELPERSISTENCE_UNLOGGED: + case RELPERSISTENCE_PERMANENT: + backend = InvalidBackendId; + break; + default: + elog(ERROR, "invalid relpersistence: %c", relpersistence); + return InvalidOid; /* placate compiler */ + } + + /* This logic should match RelationInitPhysicalAddr */ + rnode.node.spcNode = reltablespace ? reltablespace : MyDatabaseTableSpace; + rnode.node.dbNode = (rnode.node.spcNode == GLOBALTABLESPACE_OID) ? InvalidOid : MyDatabaseId; + + /* + * The relpath will vary based on the backend ID, so we must initialize + * that properly here to make sure that any collisions based on filename + * are properly detected. + */ + rnode.backend = backend; + + do + { + CHECK_FOR_INTERRUPTS(); + + /* Generate the OID */ + if (pg_class) + { + rnode.node.relNode = GetNewOidWithIndex(pg_class, ClassOidIndexId, + Anum_pg_class_oid); + } + else + { rnode.node.relNode = GetNewObjectId(); + } /* Check for existing file of same name */ rpath = relpath(rnode, MAIN_FORKNUM); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 2df76877044..c476561260c 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -950,8 +950,16 @@ index_create(Relation heapRelation, } else { - indexRelationId = - GetNewRelFileNode(tableSpaceId, pg_class, relpersistence); + if (is_enr) + { + /* Index OIDs must be kept in normal OID range. */ + indexRelationId = GetNewPermanentRelFileNode(tableSpaceId, pg_class, relpersistence); + } + else + { + indexRelationId = + GetNewRelFileNode(tableSpaceId, pg_class, relpersistence); + } } } diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index cf419a6962c..c11eb262b31 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -199,7 +199,13 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, * Create the toast table and its index */ if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTableVariable(rel)) + { pg_toast_prefix = "@pg_toast"; + } + else if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTempTable(rel) && get_ENR_withoid(currentQueryEnv, rel->rd_id, ENR_TSQL_TEMP)) + { + pg_toast_prefix = "#pg_toast"; + } snprintf(toast_relname, sizeof(toast_relname), "%s_%u", pg_toast_prefix, relOid); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index feef008199c..ee21fe83237 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -694,6 +694,12 @@ int ssl_renegotiation_limit; int huge_pages; int huge_page_size; +/* + * OIDs are stored as uint32, so we will add INT_MIN to match the range. + */ +int temp_oid_buffer_start; +int temp_oid_buffer_size; + /* * These variables are all dummies that don't do anything, except in some * cases provide the value for SHOW to display. The real state is elsewhere diff --git a/src/backend/utils/misc/queryenvironment.c b/src/backend/utils/misc/queryenvironment.c index 786f17bf681..3c2fa239fb0 100644 --- a/src/backend/utils/misc/queryenvironment.c +++ b/src/backend/utils/misc/queryenvironment.c @@ -878,15 +878,15 @@ static bool _ENR_tuple_operation(Relation catalog_rel, HeapTuple tup, ENRTupleOp CacheInvalidateHeapTuple(catalog_rel, newtup, NULL); break; case ENR_OP_UPDATE: - oldtup = lfirst(lc); - lfirst(lc) = heap_copytuple(tup); - CacheInvalidateHeapTuple(catalog_rel, oldtup, tup); + oldtup = lfirst(lc); + lfirst(lc) = heap_copytuple(tup); + CacheInvalidateHeapTuple(catalog_rel, oldtup, tup); break; case ENR_OP_DROP: - tmp = lfirst(lc); - *list_ptr = list_delete_ptr(*list_ptr, tmp); - CacheInvalidateHeapTuple(catalog_rel, tup, NULL); - heap_freetuple(tmp); + tmp = lfirst(lc); + *list_ptr = list_delete_ptr(*list_ptr, tmp); + CacheInvalidateHeapTuple(catalog_rel, tup, NULL); + heap_freetuple(tmp); break; default: break; diff --git a/src/include/access/transam.h b/src/include/access/transam.h index a0d1fb3c9f0..d09bf83ef9b 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -252,6 +252,13 @@ typedef struct VariableCacheData */ TransactionId oldestClogXid; /* oldest it's safe to look up in clog */ + /* + * These fields are also protected by OidGenLock. For tempOidStart, Shmem will + * be the source of truth, as another process may have gotten there first and + * updated the start. For tempOidBufferSize, the GUC will be the source of truth. + */ + Oid tempOidStart; + uint32 tempOidBufferSize; } VariableCacheData; typedef VariableCacheData *VariableCache; @@ -267,6 +274,7 @@ extern bool TransactionStartedDuringRecovery(void); /* in transam/varsup.c */ extern PGDLLIMPORT VariableCache ShmemVariableCache; +extern PGDLLIMPORT VariableCache TempVariableCache; /* * prototypes for functions in transam/transam.c @@ -292,6 +300,7 @@ extern void SetTransactionIdLimit(TransactionId oldest_datfrozenxid, Oid oldest_datoid); extern void AdvanceOldestClogXid(TransactionId oldest_datfrozenxid); extern bool ForceTransactionIdLimitUpdate(void); +extern Oid GetNewTempObjectId(void); extern Oid GetNewObjectId(void); extern void StopGeneratingPinnedObjectIds(void); diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h index 5b2a8cc1940..57de8e3cc23 100644 --- a/src/include/catalog/catalog.h +++ b/src/include/catalog/catalog.h @@ -36,10 +36,14 @@ extern bool IsSharedRelation(Oid relationId); extern bool IsPinnedObject(Oid classId, Oid objectId); +extern Oid GetNewTempOidWithIndex(Relation relation, Oid indexId, + AttrNumber oidcolumn); extern Oid GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn); extern Oid GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence); +extern Oid GetNewPermanentRelFileNode(Oid reltablespace, Relation pg_class, + char relpersistence); typedef bool (*IsExtendedCatalogHookType) (Oid relationId); extern IsExtendedCatalogHookType IsExtendedCatalogHook; diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 75cb2ad9ba6..8b01bc18bec 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -279,6 +279,9 @@ extern PGDLLIMPORT int temp_file_limit; extern PGDLLIMPORT int num_temp_buffers; +extern PGDLLIMPORT int temp_oid_buffer_start; +extern PGDLLIMPORT int temp_oid_buffer_size; + extern PGDLLIMPORT char *cluster_name; extern PGDLLIMPORT char *ConfigFileName; extern PGDLLIMPORT char *HbaFileName; diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index 659be74fe77..c6bd00ccf6a 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -700,4 +700,12 @@ extern void RelationDecrementReferenceCount(Relation rel); strlen((relation)->rd_rel->relname.data) >= 1 && \ (relation)->rd_rel->relname.data[0] == '@') +/* PG Tables can begin with '#' as well, so check sql_dialect additionally */ +#define RelationIsBBFTempTable(relation) \ + ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \ + (relation)->rd_rel->relname.data && \ + strlen((relation)->rd_rel->relname.data) >= 1 && \ + (relation)->rd_rel->relname.data[0] == '#') + #endif /* REL_H */ + From d57923d347b5a982ebed49b3895e54c60e72c2eb Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Mon, 11 Dec 2023 05:10:44 +0000 Subject: [PATCH 02/13] Fix ALTER TABLE with TOAST issue --- src/backend/catalog/pg_depend.c | 3 ++- src/backend/commands/cluster.c | 11 ++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 89bbb5c9e48..2bdae66e821 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -326,7 +326,8 @@ deleteDependencyRecordsFor(Oid classId, Oid objectId, ((Form_pg_depend) GETSTRUCT(tup))->deptype == DEPENDENCY_EXTENSION) continue; - CatalogTupleDelete(depRel, &tup->t_self); + if (!ENRdropTuple(depRel, tup)) + CatalogTupleDelete(depRel, &tup->t_self); count++; } diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 1dd5130d27a..85078165721 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -735,7 +735,14 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod, * mapped. This simplifies swap_relation_files, and is absolutely * necessary for rebuilding pg_class, for reasons explained there. */ - snprintf(NewHeapName, sizeof(NewHeapName), "pg_temp_%u", OIDOldHeap); + if (sql_dialect == SQL_DIALECT_TSQL && relpersistence == RELPERSISTENCE_TEMP) + { + snprintf(NewHeapName, sizeof(NewHeapName), "#pg_temp_%u", OIDOldHeap); + } + else + { + snprintf(NewHeapName, sizeof(NewHeapName), "pg_temp_%u", OIDOldHeap); + } OIDNewHeap = heap_create_with_catalog(NewHeapName, namespaceid, @@ -1603,6 +1610,8 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, /* rename the toast table ... */ if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTableVariable(newrel)) pg_toast_prefix = "@pg_toast"; + else if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTempTable(newrel) && get_ENR_withoid(currentQueryEnv, newrel->rd_id, ENR_TSQL_TEMP)) + pg_toast_prefix = "#pg_toast"; snprintf(NewToastName, NAMEDATALEN, "%s_%u", pg_toast_prefix, OIDOldHeap); From 23b77dac21f66fdc7d53f8cccda13c1e0bc21254 Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Tue, 12 Dec 2023 08:47:04 +0000 Subject: [PATCH 03/13] Minor whitespace + comment Signed-off-by: Tim Chang --- src/backend/commands/cluster.c | 3 +++ src/backend/utils/misc/guc.c | 2 +- src/backend/utils/misc/queryenvironment.c | 14 +++++++------- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 85078165721..d2cf7c56e72 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -734,6 +734,9 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod, * a shared rel. However, we do make the new heap mapped if the source is * mapped. This simplifies swap_relation_files, and is absolutely * necessary for rebuilding pg_class, for reasons explained there. + * + * We also must ensure that these temp tables are properly named in TSQL + * so that the metadata is properly cleaned up after in this function. */ if (sql_dialect == SQL_DIALECT_TSQL && relpersistence == RELPERSISTENCE_TEMP) { diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index ee21fe83237..a89cd39eb23 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -697,7 +697,7 @@ int huge_page_size; /* * OIDs are stored as uint32, so we will add INT_MIN to match the range. */ -int temp_oid_buffer_start; +int temp_oid_buffer_start; int temp_oid_buffer_size; /* diff --git a/src/backend/utils/misc/queryenvironment.c b/src/backend/utils/misc/queryenvironment.c index 3c2fa239fb0..786f17bf681 100644 --- a/src/backend/utils/misc/queryenvironment.c +++ b/src/backend/utils/misc/queryenvironment.c @@ -878,15 +878,15 @@ static bool _ENR_tuple_operation(Relation catalog_rel, HeapTuple tup, ENRTupleOp CacheInvalidateHeapTuple(catalog_rel, newtup, NULL); break; case ENR_OP_UPDATE: - oldtup = lfirst(lc); - lfirst(lc) = heap_copytuple(tup); - CacheInvalidateHeapTuple(catalog_rel, oldtup, tup); + oldtup = lfirst(lc); + lfirst(lc) = heap_copytuple(tup); + CacheInvalidateHeapTuple(catalog_rel, oldtup, tup); break; case ENR_OP_DROP: - tmp = lfirst(lc); - *list_ptr = list_delete_ptr(*list_ptr, tmp); - CacheInvalidateHeapTuple(catalog_rel, tup, NULL); - heap_freetuple(tmp); + tmp = lfirst(lc); + *list_ptr = list_delete_ptr(*list_ptr, tmp); + CacheInvalidateHeapTuple(catalog_rel, tup, NULL); + heap_freetuple(tmp); break; default: break; From 78568ccf4d9b770ad153eeeceb29ae1321bca55f Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Thu, 14 Dec 2023 22:31:39 +0000 Subject: [PATCH 04/13] Ensure non-ENR temp works as expected --- src/backend/commands/cluster.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index d2cf7c56e72..75da19a978a 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -738,7 +738,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod, * We also must ensure that these temp tables are properly named in TSQL * so that the metadata is properly cleaned up after in this function. */ - if (sql_dialect == SQL_DIALECT_TSQL && relpersistence == RELPERSISTENCE_TEMP) + if (sql_dialect == SQL_DIALECT_TSQL && relpersistence == RELPERSISTENCE_TEMP && get_ENR_withoid(currentQueryEnv, OIDOldHeap, ENR_TSQL_TEMP)) { snprintf(NewHeapName, sizeof(NewHeapName), "#pg_temp_%u", OIDOldHeap); } From 020f36a7c8f080d5f6b279c7c6a88cb4977ff6f0 Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Thu, 21 Dec 2023 05:48:14 +0000 Subject: [PATCH 05/13] Disable pgstat for temp tables to avoid concurrency issues Signed-off-by: Tim Chang --- src/backend/utils/activity/pgstat_relation.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c index a846d9ffb65..98f240ed3fa 100644 --- a/src/backend/utils/activity/pgstat_relation.c +++ b/src/backend/utils/activity/pgstat_relation.c @@ -26,6 +26,9 @@ #include "utils/rel.h" #include "utils/timestamp.h" +#include "parser/parser.h" +#include "utils/queryenvironment.h" + /* Record that's written to 2PC state file when pgstat state is persisted */ typedef struct TwoPhasePgStatRecord @@ -168,6 +171,9 @@ pgstat_unlink_relation(Relation rel) void pgstat_create_relation(Relation rel) { + /* Skip pg_stat */ + if (sql_dialect == SQL_DIALECT_TSQL && rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP) + return; pgstat_create_transactional(PGSTAT_KIND_RELATION, rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId, RelationGetRelid(rel)); @@ -182,6 +188,8 @@ pgstat_drop_relation(Relation rel) int nest_level = GetCurrentTransactionNestLevel(); PgStat_TableStatus *pgstat_info; + if (sql_dialect == SQL_DIALECT_TSQL && rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP) + return; pgstat_drop_transactional(PGSTAT_KIND_RELATION, rel->rd_rel->relisshared ? InvalidOid : MyDatabaseId, RelationGetRelid(rel)); From 350bac4b521355e8d111771af1e7fb567508fd97 Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Mon, 8 Jan 2024 05:07:13 +0000 Subject: [PATCH 06/13] Moved new OID generation functions into extension hooks Signed-off-by: Tim Chang --- src/backend/access/transam/varsup.c | 71 ------------ src/backend/catalog/catalog.c | 170 ++-------------------------- src/backend/catalog/index.c | 4 +- src/backend/catalog/toasting.c | 2 +- src/backend/commands/cluster.c | 2 +- src/include/access/transam.h | 2 +- src/include/catalog/catalog.h | 9 ++ src/include/utils/rel.h | 7 -- 8 files changed, 24 insertions(+), 243 deletions(-) diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index 640d4c183ae..ef98e96bc86 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -520,77 +520,6 @@ ForceTransactionIdLimitUpdate(void) return false; } -Oid -GetNewTempObjectId(void) -{ - Oid result; - Oid tempOidStart; - static Oid nextTempOid = InvalidOid; - - /* safety check, we should never get this far in a HS standby */ - if (RecoveryInProgress()) - elog(ERROR, "cannot assign OIDs during recovery"); - - if (!OidIsValid((Oid) temp_oid_buffer_start)) /* InvalidOid means it needs assigning */ - { - /* First check to see if another connection has already picked a start, then update. */ - LWLockAcquire(OidGenLock, LW_EXCLUSIVE); - if (OidIsValid(ShmemVariableCache->tempOidStart)) - { - temp_oid_buffer_start = OID_TO_BUFFER_START(ShmemVariableCache->tempOidStart); - nextTempOid = ShmemVariableCache->tempOidStart; - } - else - { - /* We need to pick a new start for the buffer range. */ - tempOidStart = ShmemVariableCache->nextOid; - - /* - * Decrement ShmemVariableCache->oidCount to take into account the new buffer we're allocating - */ - if (ShmemVariableCache->oidCount < temp_oid_buffer_size) - ShmemVariableCache->oidCount = 0; - else - ShmemVariableCache->oidCount -= temp_oid_buffer_size; - - /* - * If ShmemVariableCache->nextOid is below FirstNormalObjectId then we can start at FirstNormalObjectId here and - * GetNewObjectId will return the right value on the next call. - */ - if (tempOidStart < FirstNormalObjectId) - tempOidStart = FirstNormalObjectId; - - /* If the OID range would wraparound, start from beginning instead. */ - if (tempOidStart + temp_oid_buffer_size < tempOidStart) - tempOidStart = FirstNormalObjectId; - - temp_oid_buffer_start = OID_TO_BUFFER_START(tempOidStart); - ShmemVariableCache->tempOidStart = tempOidStart; - ShmemVariableCache->tempOidBufferSize = temp_oid_buffer_size; - - nextTempOid = (Oid) tempOidStart; - - /* Skip nextOid ahead to end of range here as well. */ - ShmemVariableCache->nextOid = (Oid) (tempOidStart + temp_oid_buffer_size); - } - LWLockRelease(OidGenLock); - } - - /* - * Check for wraparound of the temp OID buffer. - */ - if (nextTempOid >= (Oid) (BUFFER_START_TO_OID + temp_oid_buffer_size) - || nextTempOid < BUFFER_START_TO_OID) - { - nextTempOid = BUFFER_START_TO_OID; - } - - result = nextTempOid; - nextTempOid++; - - return result; -} - /* * GetNewObjectId -- allocate a new OID * diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 385bf25f2c3..fb0bbef62e2 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -54,6 +54,10 @@ IsExtendedCatalogHookType IsExtendedCatalogHook; IsToastRelationHookType IsToastRelationHook; IsToastClassHookType IsToastClassHook; +GetNewTempObjectId_hook_type GetNewTempObjectId_hook; +GetNewTempOidWithIndex_hook_type GetNewTempOidWithIndex_hook; +GetNewPermanentRelFileNode_hook_type GetNewPermanentRelFileNode_hook; + /* * Parameters to determine when to emit a log message in * GetNewOidWithIndex() @@ -497,76 +501,6 @@ GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn) return newOid; } -Oid -GetNewTempOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn) -{ - Oid newOid; - SysScanDesc scan; - ScanKeyData key; - bool collides; - uint64 retries = 0; - - /* Only system relations are supported */ - Assert(IsSystemRelation(relation)); - - /* In bootstrap mode, we don't have any indexes to use. No temp objects in bootstrap mode. */ - if (IsBootstrapProcessingMode()) - return GetNewObjectId(); - - /* - * We should never be asked to generate a new pg_type OID during - * pg_upgrade; doing so would risk collisions with the OIDs it wants to - * assign. Hitting this assert means there's some path where we failed to - * ensure that a type OID is determined by commands in the dump script. - */ - Assert(!IsBinaryUpgrade || RelationGetRelid(relation) != TypeRelationId); - - /* Generate new OIDs until we find one not in the table */ - do - { - CHECK_FOR_INTERRUPTS(); - - newOid = GetNewTempObjectId(); - - ScanKeyInit(&key, - oidcolumn, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(newOid)); - - /* see notes above about using SnapshotAny */ - scan = systable_beginscan(relation, indexId, true, - SnapshotAny, 1, &key); - - collides = HeapTupleIsValid(systable_getnext(scan)); - - systable_endscan(scan); - - /* - * Log that we iterate more than GETNEWOID_LOG_THRESHOLD but have not - * yet found OID unused in the relation. Then repeat logging with - * exponentially increasing intervals until we iterate more than - * GETNEWOID_LOG_MAX_INTERVAL. Finally repeat logging every - * GETNEWOID_LOG_MAX_INTERVAL unless an unused OID is found. This - * logic is necessary not to fill up the server log with the similar - * messages. - */ - if (OidIsValid((Oid) temp_oid_buffer_start) && retries >= temp_oid_buffer_size) - { - ereport(ERROR, - (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); - } - else if (OidIsValid((Oid) temp_oid_buffer_start) && retries >= (0.8 * temp_oid_buffer_size)) - { - ereport(WARNING, - (errmsg("Temp object OID usage is over 80%%. Consider dropping some temp tables or starting a new session."))); - } - - retries++; - } while (collides); - - return newOid; -} - /* * GetNewRelFileNode * Generate a new relfilenode number that is unique within the @@ -631,10 +565,10 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) /* Generate the OID */ if (pg_class) { - if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL) + if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempOidWithIndex_hook && temp_oid_buffer_size > 0) { /* Temp tables use temp OID logic */ - rnode.node.relNode = GetNewTempOidWithIndex(pg_class, ClassOidIndexId, + rnode.node.relNode = GetNewTempOidWithIndex_hook(pg_class, ClassOidIndexId, Anum_pg_class_oid); } else @@ -645,10 +579,10 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) } else { - if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL) + if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempObjectId_hook && temp_oid_buffer_size > 0) { /* Temp tables use temp OID logic */ - rnode.node.relNode = GetNewTempObjectId(); + rnode.node.relNode = GetNewTempObjectId_hook(); } else { @@ -676,12 +610,12 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) collides = false; } - if (OidIsValid((Oid) temp_oid_buffer_start) && tries > temp_oid_buffer_size) + if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && tries > temp_oid_buffer_size) { ereport(ERROR, (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); } - else if (OidIsValid((Oid) temp_oid_buffer_start) && tries >= (0.8 * temp_oid_buffer_size)) + else if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && tries >= (0.8 * temp_oid_buffer_size)) { ereport(WARNING, (errmsg("Temp object OID usage is over 80%%. Consider dropping some temp tables or starting a new session."))); @@ -694,90 +628,6 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) return rnode.node.relNode; } -/* - * Until index_create properly uses the temp OID buffer, we need a workaround to provide - * the expected BackendId while not using the temp OID buffer. */ -Oid -GetNewPermanentRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) -{ - RelFileNodeBackend rnode; - char *rpath; - bool collides; - BackendId backend; - - /* - * If we ever get here during pg_upgrade, there's something wrong; all - * relfilenode assignments during a binary-upgrade run should be - * determined by commands in the dump script. - */ - Assert(!IsBinaryUpgrade); - - switch (relpersistence) - { - case RELPERSISTENCE_TEMP: - backend = BackendIdForTempRelations(); - break; - case RELPERSISTENCE_UNLOGGED: - case RELPERSISTENCE_PERMANENT: - backend = InvalidBackendId; - break; - default: - elog(ERROR, "invalid relpersistence: %c", relpersistence); - return InvalidOid; /* placate compiler */ - } - - /* This logic should match RelationInitPhysicalAddr */ - rnode.node.spcNode = reltablespace ? reltablespace : MyDatabaseTableSpace; - rnode.node.dbNode = (rnode.node.spcNode == GLOBALTABLESPACE_OID) ? InvalidOid : MyDatabaseId; - - /* - * The relpath will vary based on the backend ID, so we must initialize - * that properly here to make sure that any collisions based on filename - * are properly detected. - */ - rnode.backend = backend; - - do - { - CHECK_FOR_INTERRUPTS(); - - /* Generate the OID */ - if (pg_class) - { - rnode.node.relNode = GetNewOidWithIndex(pg_class, ClassOidIndexId, - Anum_pg_class_oid); - } - else - { - rnode.node.relNode = GetNewObjectId(); - } - - /* Check for existing file of same name */ - rpath = relpath(rnode, MAIN_FORKNUM); - - if (access(rpath, F_OK) == 0) - { - /* definite collision */ - collides = true; - } - else - { - /* - * Here we have a little bit of a dilemma: if errno is something - * other than ENOENT, should we declare a collision and loop? In - * practice it seems best to go ahead regardless of the errno. If - * there is a colliding file we will get an smgr failure when we - * attempt to create the new relation file. - */ - collides = false; - } - - pfree(rpath); - } while (collides); - - return rnode.node.relNode; -} - /* * SQL callable interface for GetNewOidWithIndex(). Outside of initdb's * direct insertions into catalog tables, and recovering from corruption, this diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index c476561260c..d6de0ea7a00 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -950,10 +950,10 @@ index_create(Relation heapRelation, } else { - if (is_enr) + if (is_enr && GetNewPermanentRelFileNode_hook) { /* Index OIDs must be kept in normal OID range. */ - indexRelationId = GetNewPermanentRelFileNode(tableSpaceId, pg_class, relpersistence); + indexRelationId = GetNewPermanentRelFileNode_hook(tableSpaceId, pg_class, relpersistence); } else { diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index c11eb262b31..6fa36b7b733 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -202,7 +202,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, { pg_toast_prefix = "@pg_toast"; } - else if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTempTable(rel) && get_ENR_withoid(currentQueryEnv, rel->rd_id, ENR_TSQL_TEMP)) + else if (sql_dialect == SQL_DIALECT_TSQL && rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP && get_ENR_withoid(currentQueryEnv, rel->rd_id, ENR_TSQL_TEMP)) { pg_toast_prefix = "#pg_toast"; } diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 75da19a978a..947d4f64e22 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1613,7 +1613,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, /* rename the toast table ... */ if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTableVariable(newrel)) pg_toast_prefix = "@pg_toast"; - else if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTempTable(newrel) && get_ENR_withoid(currentQueryEnv, newrel->rd_id, ENR_TSQL_TEMP)) + else if (sql_dialect == SQL_DIALECT_TSQL && newrel->rd_rel->relpersistence == RELPERSISTENCE_TEMP && get_ENR_withoid(currentQueryEnv, newrel->rd_id, ENR_TSQL_TEMP)) pg_toast_prefix = "#pg_toast"; snprintf(NewToastName, NAMEDATALEN, "%s_%u", diff --git a/src/include/access/transam.h b/src/include/access/transam.h index d09bf83ef9b..89b8f623a12 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -255,7 +255,7 @@ typedef struct VariableCacheData /* * These fields are also protected by OidGenLock. For tempOidStart, Shmem will * be the source of truth, as another process may have gotten there first and - * updated the start. For tempOidBufferSize, the GUC will be the source of truth. + * updated the start. */ Oid tempOidStart; uint32 tempOidBufferSize; diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h index 57de8e3cc23..5068a2fa5ea 100644 --- a/src/include/catalog/catalog.h +++ b/src/include/catalog/catalog.h @@ -45,6 +45,15 @@ extern Oid GetNewRelFileNode(Oid reltablespace, Relation pg_class, extern Oid GetNewPermanentRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence); +typedef Oid (*GetNewTempObjectId_hook_type) (void); +extern GetNewTempObjectId_hook_type GetNewTempObjectId_hook; + +typedef Oid (*GetNewTempOidWithIndex_hook_type) (Relation relation, Oid indexId, AttrNumber oidcolumn); +extern GetNewTempOidWithIndex_hook_type GetNewTempOidWithIndex_hook; + +typedef Oid (*GetNewPermanentRelFileNode_hook_type) (Oid reltablespace, Relation pg_class, char relpersistence); +extern GetNewPermanentRelFileNode_hook_type GetNewPermanentRelFileNode_hook; + typedef bool (*IsExtendedCatalogHookType) (Oid relationId); extern IsExtendedCatalogHookType IsExtendedCatalogHook; diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index c6bd00ccf6a..be030d0f947 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -700,12 +700,5 @@ extern void RelationDecrementReferenceCount(Relation rel); strlen((relation)->rd_rel->relname.data) >= 1 && \ (relation)->rd_rel->relname.data[0] == '@') -/* PG Tables can begin with '#' as well, so check sql_dialect additionally */ -#define RelationIsBBFTempTable(relation) \ - ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP && \ - (relation)->rd_rel->relname.data && \ - strlen((relation)->rd_rel->relname.data) >= 1 && \ - (relation)->rd_rel->relname.data[0] == '#') - #endif /* REL_H */ From cfb4d119053146144a8aa20b0567f2d327aafc6c Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Mon, 8 Jan 2024 05:11:23 +0000 Subject: [PATCH 07/13] Minor style fixes Signed-off-by: Tim Chang --- src/backend/access/transam/varsup.c | 4 +--- src/backend/catalog/catalog.c | 16 ++-------------- src/backend/catalog/index.c | 6 +----- src/backend/catalog/toasting.c | 4 ---- src/include/utils/rel.h | 1 - 5 files changed, 4 insertions(+), 27 deletions(-) diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index ef98e96bc86..2e8ff509dc2 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -33,9 +33,6 @@ /* Number of OIDs to prefetch (preallocate) per XLOG write */ #define VAR_OID_PREFETCH 8192 -#define OID_TO_BUFFER_START(oid) ((oid) + INT_MIN) -#define BUFFER_START_TO_OID ((Oid) (temp_oid_buffer_start) - INT_MIN) - /* pointer to "variable cache" in shared memory (set up by shmem.c) */ VariableCache ShmemVariableCache = NULL; @@ -520,6 +517,7 @@ ForceTransactionIdLimitUpdate(void) return false; } + /* * GetNewObjectId -- allocate a new OID * diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index fb0bbef62e2..17ee2b22954 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -565,29 +565,21 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) /* Generate the OID */ if (pg_class) { + /* Temp tables use temp OID logic */ if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempOidWithIndex_hook && temp_oid_buffer_size > 0) - { - /* Temp tables use temp OID logic */ rnode.node.relNode = GetNewTempOidWithIndex_hook(pg_class, ClassOidIndexId, Anum_pg_class_oid); - } else - { rnode.node.relNode = GetNewOidWithIndex(pg_class, ClassOidIndexId, Anum_pg_class_oid); - } } else { + /* Temp tables use temp OID logic */ if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempObjectId_hook && temp_oid_buffer_size > 0) - { - /* Temp tables use temp OID logic */ rnode.node.relNode = GetNewTempObjectId_hook(); - } else - { rnode.node.relNode = GetNewObjectId(); - } } /* Check for existing file of same name */ @@ -611,15 +603,11 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) } if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && tries > temp_oid_buffer_size) - { ereport(ERROR, (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); - } else if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && tries >= (0.8 * temp_oid_buffer_size)) - { ereport(WARNING, (errmsg("Temp object OID usage is over 80%%. Consider dropping some temp tables or starting a new session."))); - } pfree(rpath); tries++; diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index d6de0ea7a00..10a5f6d0c08 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -950,16 +950,12 @@ index_create(Relation heapRelation, } else { + /* Index OIDs must be kept in normal OID range. */ if (is_enr && GetNewPermanentRelFileNode_hook) - { - /* Index OIDs must be kept in normal OID range. */ indexRelationId = GetNewPermanentRelFileNode_hook(tableSpaceId, pg_class, relpersistence); - } else - { indexRelationId = GetNewRelFileNode(tableSpaceId, pg_class, relpersistence); - } } } diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 6fa36b7b733..4afde261b43 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -199,13 +199,9 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, * Create the toast table and its index */ if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTableVariable(rel)) - { pg_toast_prefix = "@pg_toast"; - } else if (sql_dialect == SQL_DIALECT_TSQL && rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP && get_ENR_withoid(currentQueryEnv, rel->rd_id, ENR_TSQL_TEMP)) - { pg_toast_prefix = "#pg_toast"; - } snprintf(toast_relname, sizeof(toast_relname), "%s_%u", pg_toast_prefix, relOid); diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index be030d0f947..659be74fe77 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -701,4 +701,3 @@ extern void RelationDecrementReferenceCount(Relation rel); (relation)->rd_rel->relname.data[0] == '@') #endif /* REL_H */ - From 36692f70578ff551e725238100252264e42465cc Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Mon, 8 Jan 2024 18:53:40 +0000 Subject: [PATCH 08/13] Additional style fixed, removed obsolete vars Signed-off-by: Tim Chang --- src/backend/catalog/catalog.c | 13 +++++++++---- src/backend/commands/cluster.c | 4 ---- src/include/access/transam.h | 2 -- src/include/catalog/catalog.h | 4 ---- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 17ee2b22954..41551132f38 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -58,6 +58,11 @@ GetNewTempObjectId_hook_type GetNewTempObjectId_hook; GetNewTempOidWithIndex_hook_type GetNewTempOidWithIndex_hook; GetNewPermanentRelFileNode_hook_type GetNewPermanentRelFileNode_hook; +/* + * Temp tables in BBF should use the OID buffer. + */ +#define USE_BBF_OID_BUFFER (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempOidWithIndex_hook && temp_oid_buffer_size > 0) + /* * Parameters to determine when to emit a log message in * GetNewOidWithIndex() @@ -566,7 +571,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) if (pg_class) { /* Temp tables use temp OID logic */ - if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempOidWithIndex_hook && temp_oid_buffer_size > 0) + if (USE_BBF_OID_BUFFER) rnode.node.relNode = GetNewTempOidWithIndex_hook(pg_class, ClassOidIndexId, Anum_pg_class_oid); else @@ -576,7 +581,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) else { /* Temp tables use temp OID logic */ - if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempObjectId_hook && temp_oid_buffer_size > 0) + if (USE_BBF_OID_BUFFER) rnode.node.relNode = GetNewTempObjectId_hook(); else rnode.node.relNode = GetNewObjectId(); @@ -602,10 +607,10 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) collides = false; } - if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && tries > temp_oid_buffer_size) + if (USE_BBF_OID_BUFFER && tries > temp_oid_buffer_size) ereport(ERROR, (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); - else if (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && tries >= (0.8 * temp_oid_buffer_size)) + else if (USE_BBF_OID_BUFFER && tries >= (0.8 * temp_oid_buffer_size)) ereport(WARNING, (errmsg("Temp object OID usage is over 80%%. Consider dropping some temp tables or starting a new session."))); diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 947d4f64e22..6448add1000 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -739,13 +739,9 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod, * so that the metadata is properly cleaned up after in this function. */ if (sql_dialect == SQL_DIALECT_TSQL && relpersistence == RELPERSISTENCE_TEMP && get_ENR_withoid(currentQueryEnv, OIDOldHeap, ENR_TSQL_TEMP)) - { snprintf(NewHeapName, sizeof(NewHeapName), "#pg_temp_%u", OIDOldHeap); - } else - { snprintf(NewHeapName, sizeof(NewHeapName), "pg_temp_%u", OIDOldHeap); - } OIDNewHeap = heap_create_with_catalog(NewHeapName, namespaceid, diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 89b8f623a12..c3e70d31094 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -274,7 +274,6 @@ extern bool TransactionStartedDuringRecovery(void); /* in transam/varsup.c */ extern PGDLLIMPORT VariableCache ShmemVariableCache; -extern PGDLLIMPORT VariableCache TempVariableCache; /* * prototypes for functions in transam/transam.c @@ -300,7 +299,6 @@ extern void SetTransactionIdLimit(TransactionId oldest_datfrozenxid, Oid oldest_datoid); extern void AdvanceOldestClogXid(TransactionId oldest_datfrozenxid); extern bool ForceTransactionIdLimitUpdate(void); -extern Oid GetNewTempObjectId(void); extern Oid GetNewObjectId(void); extern void StopGeneratingPinnedObjectIds(void); diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h index 5068a2fa5ea..536f64b85d7 100644 --- a/src/include/catalog/catalog.h +++ b/src/include/catalog/catalog.h @@ -36,14 +36,10 @@ extern bool IsSharedRelation(Oid relationId); extern bool IsPinnedObject(Oid classId, Oid objectId); -extern Oid GetNewTempOidWithIndex(Relation relation, Oid indexId, - AttrNumber oidcolumn); extern Oid GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn); extern Oid GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence); -extern Oid GetNewPermanentRelFileNode(Oid reltablespace, Relation pg_class, - char relpersistence); typedef Oid (*GetNewTempObjectId_hook_type) (void); extern GetNewTempObjectId_hook_type GetNewTempObjectId_hook; From d275788acb26f5aa4200cc22d469f93b6bbd8495 Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Mon, 8 Jan 2024 20:27:01 +0000 Subject: [PATCH 09/13] Remove 80% warning since it is misleading Signed-off-by: Tim Chang --- src/backend/catalog/catalog.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 41551132f38..e7173f2783e 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -610,9 +610,6 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) if (USE_BBF_OID_BUFFER && tries > temp_oid_buffer_size) ereport(ERROR, (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); - else if (USE_BBF_OID_BUFFER && tries >= (0.8 * temp_oid_buffer_size)) - ereport(WARNING, - (errmsg("Temp object OID usage is over 80%%. Consider dropping some temp tables or starting a new session."))); pfree(rpath); tries++; From 6f362009e9476d29db7e32a7c968f56a881a9dc0 Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Wed, 10 Jan 2024 19:45:55 +0000 Subject: [PATCH 10/13] Minor fixes Signed-off-by: Tim Chang --- src/backend/access/transam/varsup.c | 3 --- src/backend/catalog/catalog.c | 18 ++++++++---------- src/backend/catalog/heap.c | 2 +- src/backend/catalog/index.c | 12 ++++++------ src/backend/commands/tablecmds.c | 2 +- src/backend/utils/cache/relcache.c | 2 +- src/include/access/transam.h | 3 +-- src/include/catalog/catalog.h | 5 +---- 8 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index 2e8ff509dc2..dd3d7498250 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -13,8 +13,6 @@ #include "postgres.h" -#include - #include "access/clog.h" #include "access/commit_ts.h" #include "access/subtrans.h" @@ -26,7 +24,6 @@ #include "postmaster/autovacuum.h" #include "storage/pmsignal.h" #include "storage/proc.h" -#include "utils/guc.h" #include "utils/syscache.h" diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index e7173f2783e..507d5b57d72 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -56,12 +56,6 @@ IsToastClassHookType IsToastClassHook; GetNewTempObjectId_hook_type GetNewTempObjectId_hook; GetNewTempOidWithIndex_hook_type GetNewTempOidWithIndex_hook; -GetNewPermanentRelFileNode_hook_type GetNewPermanentRelFileNode_hook; - -/* - * Temp tables in BBF should use the OID buffer. - */ -#define USE_BBF_OID_BUFFER (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL && GetNewTempOidWithIndex_hook && temp_oid_buffer_size > 0) /* * Parameters to determine when to emit a log message in @@ -523,13 +517,14 @@ GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn) * created by bootstrap have preassigned OIDs, so there's no need. */ Oid -GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) +GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence, bool override_temp) { RelFileNodeBackend rnode; char *rpath; bool collides; BackendId backend; int tries = 0; + bool use_bbf_oid_buffer; /* * If we ever get here during pg_upgrade, there's something wrong; all @@ -563,6 +558,9 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) */ rnode.backend = backend; + use_bbf_oid_buffer = (relpersistence == RELPERSISTENCE_TEMP && sql_dialect == SQL_DIALECT_TSQL + && GetNewTempOidWithIndex_hook && temp_oid_buffer_size > 0 && !override_temp); + do { CHECK_FOR_INTERRUPTS(); @@ -571,7 +569,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) if (pg_class) { /* Temp tables use temp OID logic */ - if (USE_BBF_OID_BUFFER) + if (use_bbf_oid_buffer) rnode.node.relNode = GetNewTempOidWithIndex_hook(pg_class, ClassOidIndexId, Anum_pg_class_oid); else @@ -581,7 +579,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) else { /* Temp tables use temp OID logic */ - if (USE_BBF_OID_BUFFER) + if (use_bbf_oid_buffer) rnode.node.relNode = GetNewTempObjectId_hook(); else rnode.node.relNode = GetNewObjectId(); @@ -607,7 +605,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence) collides = false; } - if (USE_BBF_OID_BUFFER && tries > temp_oid_buffer_size) + if (use_bbf_oid_buffer && tries > temp_oid_buffer_size) ereport(ERROR, (errmsg("Unable to allocate oid for temp table. Drop some temporary tables or start a new session."))); diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 2c20cbe0dd5..5b03c59beb8 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1307,7 +1307,7 @@ heap_create_with_catalog(const char *relname, if (!OidIsValid(relid)) relid = GetNewRelFileNode(reltablespace, pg_class_desc, - relpersistence); + relpersistence, false); } /* diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 10a5f6d0c08..7f6e101516f 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -950,12 +950,12 @@ index_create(Relation heapRelation, } else { - /* Index OIDs must be kept in normal OID range. */ - if (is_enr && GetNewPermanentRelFileNode_hook) - indexRelationId = GetNewPermanentRelFileNode_hook(tableSpaceId, pg_class, relpersistence); - else - indexRelationId = - GetNewRelFileNode(tableSpaceId, pg_class, relpersistence); + /* + * Index OIDs must be kept in normal OID range due to deletion issues. + * Since deletion is sorted by OID, adding indexes to temp OID range + * causes deletion order issues. + */ + indexRelationId = GetNewRelFileNode(tableSpaceId, pg_class, relpersistence, is_enr); } } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b4a7d978257..88818af9874 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14582,7 +14582,7 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) * to allocate a new one in the new tablespace. */ newrelfilenode = GetNewRelFileNode(newTableSpace, NULL, - rel->rd_rel->relpersistence); + rel->rd_rel->relpersistence, false); /* Open old and new relation */ newrnode = rel->rd_node; diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 41d71abc0e6..b0126dfd5e1 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3735,7 +3735,7 @@ RelationSetNewRelfilenode(Relation relation, char persistence) { /* Allocate a new relfilenode */ newrelfilenode = GetNewRelFileNode(relation->rd_rel->reltablespace, - NULL, persistence); + NULL, persistence, false); } else if (relation->rd_rel->relkind == RELKIND_INDEX) { diff --git a/src/include/access/transam.h b/src/include/access/transam.h index c3e70d31094..45bc724ce88 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -253,12 +253,11 @@ typedef struct VariableCacheData TransactionId oldestClogXid; /* oldest it's safe to look up in clog */ /* - * These fields are also protected by OidGenLock. For tempOidStart, Shmem will + * This field is also protected by OidGenLock. For tempOidStart, Shmem will * be the source of truth, as another process may have gotten there first and * updated the start. */ Oid tempOidStart; - uint32 tempOidBufferSize; } VariableCacheData; typedef VariableCacheData *VariableCache; diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h index 536f64b85d7..5652f931f9f 100644 --- a/src/include/catalog/catalog.h +++ b/src/include/catalog/catalog.h @@ -39,7 +39,7 @@ extern bool IsPinnedObject(Oid classId, Oid objectId); extern Oid GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn); extern Oid GetNewRelFileNode(Oid reltablespace, Relation pg_class, - char relpersistence); + char relpersistence, bool override_temp); typedef Oid (*GetNewTempObjectId_hook_type) (void); extern GetNewTempObjectId_hook_type GetNewTempObjectId_hook; @@ -47,9 +47,6 @@ extern GetNewTempObjectId_hook_type GetNewTempObjectId_hook; typedef Oid (*GetNewTempOidWithIndex_hook_type) (Relation relation, Oid indexId, AttrNumber oidcolumn); extern GetNewTempOidWithIndex_hook_type GetNewTempOidWithIndex_hook; -typedef Oid (*GetNewPermanentRelFileNode_hook_type) (Oid reltablespace, Relation pg_class, char relpersistence); -extern GetNewPermanentRelFileNode_hook_type GetNewPermanentRelFileNode_hook; - typedef bool (*IsExtendedCatalogHookType) (Oid relationId); extern IsExtendedCatalogHookType IsExtendedCatalogHook; From ea15243da7099d03af0c0164d119a88c5a066536 Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Tue, 16 Jan 2024 03:40:25 +0000 Subject: [PATCH 11/13] Move macro to transam for visibility --- src/backend/access/transam/varsup.c | 3 --- src/include/access/transam.h | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index dd3d7498250..3ade84616a3 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -27,9 +27,6 @@ #include "utils/syscache.h" -/* Number of OIDs to prefetch (preallocate) per XLOG write */ -#define VAR_OID_PREFETCH 8192 - /* pointer to "variable cache" in shared memory (set up by shmem.c) */ VariableCache ShmemVariableCache = NULL; diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 45bc724ce88..9bcc7d3d91e 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -196,6 +196,9 @@ FullTransactionIdAdvance(FullTransactionId *dest) #define FirstUnpinnedObjectId 12000 #define FirstNormalObjectId 16384 +/* Number of OIDs to prefetch (preallocate) per XLOG write */ +#define VAR_OID_PREFETCH 8192 + /* * VariableCache is a data structure in shared memory that is used to track * OID and XID assignment state. For largely historical reasons, there is From c1570bc1c322ea680b3af2f777f5fa2ae6d5bfaf Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Wed, 17 Jan 2024 19:25:26 +0000 Subject: [PATCH 12/13] Revert "Move macro to transam for visibility" This reverts commit ea15243da7099d03af0c0164d119a88c5a066536. --- src/backend/access/transam/varsup.c | 3 +++ src/include/access/transam.h | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index 3ade84616a3..dd3d7498250 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -27,6 +27,9 @@ #include "utils/syscache.h" +/* Number of OIDs to prefetch (preallocate) per XLOG write */ +#define VAR_OID_PREFETCH 8192 + /* pointer to "variable cache" in shared memory (set up by shmem.c) */ VariableCache ShmemVariableCache = NULL; diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 9bcc7d3d91e..45bc724ce88 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -196,9 +196,6 @@ FullTransactionIdAdvance(FullTransactionId *dest) #define FirstUnpinnedObjectId 12000 #define FirstNormalObjectId 16384 -/* Number of OIDs to prefetch (preallocate) per XLOG write */ -#define VAR_OID_PREFETCH 8192 - /* * VariableCache is a data structure in shared memory that is used to track * OID and XID assignment state. For largely historical reasons, there is From 8d8bb1f554f45623a1bd5b3b5223490d5bfc818b Mon Sep 17 00:00:00 2001 From: Tim Chang Date: Wed, 17 Jan 2024 19:28:22 +0000 Subject: [PATCH 13/13] Add simple fetch for VAR_OID_PREFETCH --- src/backend/access/transam/varsup.c | 8 ++++++++ src/include/access/transam.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index dd3d7498250..ad39a029332 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -590,6 +590,14 @@ GetNewObjectId(void) return result; } +/* + * Simple function to get VAR_OID_PREFETCH when needed outside of this file. + */ +int GetVarOidPrefetch(void) +{ + return VAR_OID_PREFETCH; +} + /* * SetNextObjectId * diff --git a/src/include/access/transam.h b/src/include/access/transam.h index 45bc724ce88..300f76629aa 100644 --- a/src/include/access/transam.h +++ b/src/include/access/transam.h @@ -301,6 +301,8 @@ extern bool ForceTransactionIdLimitUpdate(void); extern Oid GetNewObjectId(void); extern void StopGeneratingPinnedObjectIds(void); +extern int GetVarOidPrefetch(void); + typedef void (*GetNewObjectId_hook_type) (VariableCache variableCache); extern PGDLLIMPORT GetNewObjectId_hook_type GetNewObjectId_hook;