From 3e9191e310881d6402f28080c5a41d33663e8aa8 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Mon, 14 Jul 2025 22:10:36 +0100 Subject: [PATCH] PKG-895: Allow citus and timescaledb to compile This commit reintroduces the old method signatures of some methods we changed to support pg_tde. The changed method signatures still exists with a "2" suffix, and all internal code calls the "2" methods. The reintroduced original methods also check a global flag which can disable them - if this happens, they report a FATAL error instead. This variable is activated by pg_tde, which means that if something tries to use these while pg_tde is loaded, and with that, possibly ignores its file tracking mechanism, it reports an error. In practice this means that timescaledb, or the columnar storage in citus won't work if pg_tde is loaded in the shared_preload_libraries, but they can be used with our distribution without pg_tde enabled. Also verified that both citus and timescaledb do compile with these changes, and their basic test suite can be run againts our fork. --- contrib/pg_tde/src/pg_tde.c | 2 ++ src/backend/access/heap/heapam_handler.c | 8 +++--- src/backend/access/transam/xlogutils.c | 2 +- src/backend/catalog/heap.c | 2 +- src/backend/catalog/index.c | 35 ++++++++++++++++++++++-- src/backend/catalog/storage.c | 18 +++++++++--- src/backend/catalog/toasting.c | 2 +- src/backend/commands/indexcmds.c | 2 +- src/backend/commands/sequence.c | 2 +- src/backend/commands/tablecmds.c | 4 +-- src/backend/storage/buffer/bufmgr.c | 6 ++-- src/backend/storage/smgr/smgr.c | 13 ++++++++- src/backend/utils/cache/relcache.c | 2 +- src/include/catalog/index.h | 22 +++++++++++++++ src/include/catalog/storage.h | 5 +++- src/include/storage/smgr.h | 4 ++- 16 files changed, 104 insertions(+), 25 deletions(-) diff --git a/contrib/pg_tde/src/pg_tde.c b/contrib/pg_tde/src/pg_tde.c index 626d5493b8fb3..e53712045ac90 100644 --- a/contrib/pg_tde/src/pg_tde.c +++ b/contrib/pg_tde/src/pg_tde.c @@ -101,6 +101,8 @@ _PG_init(void) shmem_request_hook = tde_shmem_request; prev_shmem_startup_hook = shmem_startup_hook; shmem_startup_hook = tde_shmem_startup; + + allow_upstream_smgr_api = false; } static void diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 87ec4a367fa8f..02ec6a7641230 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -605,7 +605,7 @@ heapam_relation_set_new_filelocator(Relation rel, */ *minmulti = GetOldestMultiXactId(); - srel = RelationCreateStorage(oldlocator, *newrlocator, persistence, true); + srel = RelationCreateStorage2(oldlocator, *newrlocator, persistence, true); /* * If required, set up an init fork for an unlogged table so that it can @@ -618,7 +618,7 @@ heapam_relation_set_new_filelocator(Relation rel, Assert(rel->rd_rel->relkind == RELKIND_RELATION || rel->rd_rel->relkind == RELKIND_MATVIEW || rel->rd_rel->relkind == RELKIND_TOASTVALUE); - smgrcreate(oldlocator, srel, INIT_FORKNUM, false); + smgrcreate2(oldlocator, srel, INIT_FORKNUM, false); log_smgrcreate(newrlocator, INIT_FORKNUM); } @@ -651,7 +651,7 @@ heapam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) * NOTE: any conflict in relfilenumber value will be caught in * RelationCreateStorage(). */ - dstrel = RelationCreateStorage(rel->rd_locator, *newrlocator, rel->rd_rel->relpersistence, true); + dstrel = RelationCreateStorage2(rel->rd_locator, *newrlocator, rel->rd_rel->relpersistence, true); /* copy main fork */ RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, @@ -663,7 +663,7 @@ heapam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) { if (smgrexists(RelationGetSmgr(rel), forkNum)) { - smgrcreate(rel->rd_locator, dstrel, forkNum, false); + smgrcreate2(rel->rd_locator, dstrel, forkNum, false); /* * WAL log creation if the relation is persistent, or this is the diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index d459e1bd7245b..603b1e456d000 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -498,7 +498,7 @@ XLogReadBufferExtended(RelFileLocator rlocator, ForkNumber forknum, * filesystem loses an inode during a crash. Better to write the data * until we are actually told to delete the file.) */ - smgrcreate(rlocator, smgr, forknum, true); + smgrcreate2(rlocator, smgr, forknum, true); lastblock = smgrnblocks(smgr, forknum); diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 71eb3b2432c48..345ee60ea67e6 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -406,7 +406,7 @@ heap_create(const char *relname, relpersistence, relfrozenxid, relminmxid); else if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind)) - RelationCreateStorage(prev_rlocator, new_rlocator, relpersistence, true); + RelationCreateStorage2(prev_rlocator, new_rlocator, relpersistence, true); else Assert(false); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 706ab4cfa6af7..466da86ef6bd4 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -672,6 +672,35 @@ UpdateIndexRelation(Oid indexoid, heap_freetuple(tuple); } +extern Oid index_create(Relation heapRelation, + const char *indexRelationName, + Oid indexRelationId, + Oid parentIndexRelid, + Oid parentConstraintId, + RelFileNumber relFileNumber, + IndexInfo *indexInfo, + const List *indexColNames, + Oid accessMethodId, + Oid tableSpaceId, + const Oid *collationIds, + const Oid *opclassIds, + const Datum *opclassOptions, + const int16 *coloptions, + const NullableDatum *stattargets, + Datum reloptions, + bits16 flags, + bits16 constr_flags, + bool allow_system_table_mods, + bool is_internal, + Oid *constraintId) +{ + if(!allow_upstream_smgr_api) { + elog(FATAL, "An extension is trying to use the traditional index_create method, while another loaded extension (pg_tde) requires the new API."); + } + return index_create2(heapRelation, indexRelationName, indexRelationId, parentIndexRelid, parentConstraintId, relFileNumber, + indexInfo, indexColNames, accessMethodId, tableSpaceId, collationIds, opclassIds, opclassOptions, coloptions, stattargets, reloptions, + flags, constr_flags, allow_system_table_mods, is_internal, constraintId, NULL); +} /* * index_create @@ -721,7 +750,7 @@ UpdateIndexRelation(Oid indexoid, * Returns the OID of the created index. */ Oid -index_create(Relation heapRelation, +index_create2(Relation heapRelation, const char *indexRelationName, Oid indexRelationId, Oid parentIndexRelid, @@ -1441,7 +1470,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, * ensure a consistent state at all times. That is why parentIndexRelid * is not set here. */ - newIndexId = index_create(heapRelation, + newIndexId = index_create2(heapRelation, newName, InvalidOid, /* indexRelationId */ InvalidOid, /* parentIndexRelid */ @@ -3030,7 +3059,7 @@ index_build(Relation heapRelation, if (indexRelation->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED && !smgrexists(RelationGetSmgr(indexRelation), INIT_FORKNUM)) { - smgrcreate(indexRelation->rd_locator, RelationGetSmgr(indexRelation), INIT_FORKNUM, false); + smgrcreate2(indexRelation->rd_locator, RelationGetSmgr(indexRelation), INIT_FORKNUM, false); log_smgrcreate(&indexRelation->rd_locator, INIT_FORKNUM); indexRelation->rd_indam->ambuildempty(indexRelation); } diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index 787198653f94b..c2fbba72bcc01 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -104,6 +104,16 @@ AddPendingSync(const RelFileLocator *rlocator) pending->is_truncated = false; } +extern SMgrRelation RelationCreateStorage(RelFileLocator rlocator, + char relpersistence, + bool register_delete) +{ + if(!allow_upstream_smgr_api) { + elog(FATAL, "An extension is trying to use the traditional RelationCreateStorage method, while another loaded extension (pg_tde) requires the new API."); + } + return RelationCreateStorage2(rlocator, rlocator, relpersistence, register_delete); +} + /* * RelationCreateStorage * Create physical storage for a relation. @@ -118,7 +128,7 @@ AddPendingSync(const RelFileLocator *rlocator) * pass register_delete = false. */ SMgrRelation -RelationCreateStorage(RelFileLocator oldlocator, RelFileLocator rlocator, char relpersistence, +RelationCreateStorage2(RelFileLocator oldlocator, RelFileLocator rlocator, char relpersistence, bool register_delete) { SMgrRelation srel; @@ -147,7 +157,7 @@ RelationCreateStorage(RelFileLocator oldlocator, RelFileLocator rlocator, char r } srel = smgropen(rlocator, procNumber); - smgrcreate(oldlocator, srel, MAIN_FORKNUM, false); + smgrcreate2(oldlocator, srel, MAIN_FORKNUM, false); if (needs_wal) log_smgrcreate(&srel->smgr_rlocator.locator, MAIN_FORKNUM); @@ -976,7 +986,7 @@ smgr_redo(XLogReaderState *record) SMgrRelation reln; reln = smgropen(xlrec->rlocator, INVALID_PROC_NUMBER); - smgrcreate(xlrec->rlocator, reln, xlrec->forkNum, true); + smgrcreate2(xlrec->rlocator, reln, xlrec->forkNum, true); } else if (info == XLOG_SMGR_TRUNCATE) { @@ -997,7 +1007,7 @@ smgr_redo(XLogReaderState *record) * XLogReadBufferForRedo, we prefer to recreate the rel and replay the * log as best we can until the drop is seen. */ - smgrcreate(xlrec->rlocator, reln, MAIN_FORKNUM, true); + smgrcreate2(xlrec->rlocator, reln, MAIN_FORKNUM, true); /* * Before we perform the truncation, update minimum recovery point to diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index fa68de27bcaad..762888700ba00 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -318,7 +318,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, coloptions[0] = 0; coloptions[1] = 0; - index_create(toast_rel, toast_idxname, toastIndexOid, InvalidOid, + index_create2(toast_rel, toast_idxname, toastIndexOid, InvalidOid, InvalidOid, InvalidOid, indexInfo, list_make2("chunk_id", "chunk_seq"), diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index ff96a56c9e9b9..f93b88170c59b 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1200,7 +1200,7 @@ DefineIndex(Oid tableId, constr_flags |= INDEX_CONSTR_CREATE_INIT_DEFERRED; indexRelationId = - index_create(rel, indexRelationName, indexRelationId, parentIndexId, + index_create2(rel, indexRelationName, indexRelationId, parentIndexId, parentConstraintId, stmt->oldNumber, indexInfo, indexColNames, accessMethodId, tablespaceId, diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index ebaee8f045d38..be3024ac41820 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -344,7 +344,7 @@ fill_seq_with_data(Relation rel, HeapTuple tuple) SMgrRelation srel; srel = smgropen(rel->rd_locator, INVALID_PROC_NUMBER); - smgrcreate(rel->rd_locator, srel, INIT_FORKNUM, false); + smgrcreate2(rel->rd_locator, srel, INIT_FORKNUM, false); log_smgrcreate(&rel->rd_locator, INIT_FORKNUM); fill_seq_fork_with_data(rel, tuple, INIT_FORKNUM); FlushRelationBuffers(rel); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b1090497eb401..c5e3ebd46e388 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -15540,7 +15540,7 @@ index_copy_data(Relation rel, RelFileLocator newrlocator) * NOTE: any conflict in relfilenumber value will be caught in * RelationCreateStorage(). */ - dstrel = RelationCreateStorage(rel->rd_locator, newrlocator, rel->rd_rel->relpersistence, true); + dstrel = RelationCreateStorage2(rel->rd_locator, newrlocator, rel->rd_rel->relpersistence, true); /* copy main fork */ RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, @@ -15552,7 +15552,7 @@ index_copy_data(Relation rel, RelFileLocator newrlocator) { if (smgrexists(RelationGetSmgr(rel), forkNum)) { - smgrcreate(rel->rd_locator, dstrel, forkNum, false); + smgrcreate2(rel->rd_locator, dstrel, forkNum, false); /* * WAL log creation if the relation is persistent, or this is the diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 922f1f4141c5f..50834f4b2311f 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -942,7 +942,7 @@ ExtendBufferedRelTo(BufferManagerRelation bmr, /* recheck, fork might have been created concurrently */ if (!smgrexists(bmr.smgr, fork)) - smgrcreate(bmr.rel->rd_locator, bmr.smgr, fork, flags & EB_PERFORMING_RECOVERY); + smgrcreate2(bmr.rel->rd_locator, bmr.smgr, fork, flags & EB_PERFORMING_RECOVERY); UnlockRelationForExtension(bmr.rel, ExclusiveLock); } @@ -4788,7 +4788,7 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator, * directory. Therefore, each individual relation doesn't need to be * registered for cleanup. */ - RelationCreateStorage(src_rlocator, dst_rlocator, relpersistence, false); + RelationCreateStorage2(src_rlocator, dst_rlocator, relpersistence, false); /* copy main fork. */ RelationCopyStorageUsingBuffer(src_rlocator, dst_rlocator, MAIN_FORKNUM, @@ -4801,7 +4801,7 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator, if (smgrexists(src_rel, forkNum)) { /* TODO: for sure? */ - smgrcreate(src_rel->smgr_rlocator.locator, dst_rel, forkNum, false); + smgrcreate2(src_rel->smgr_rlocator.locator, dst_rel, forkNum, false); /* * WAL log creation if the relation is persistent, or this is the diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index c9f32ccdb3f4b..4babd866d68d9 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -73,6 +73,8 @@ static Size LargestSMgrRelationSize = 0; char *storage_manager_string; SMgrId storage_manager_id; +bool allow_upstream_smgr_api = true; + /* * Each backend has a hashtable that stores all extant SMgrRelation objects. * In addition, "unpinned" SMgrRelation objects are chained together in a list. @@ -399,6 +401,15 @@ smgrexists(SMgrRelation reln, ForkNumber forknum) return smgrsw[reln->smgr_which].smgr_exists(reln, forknum); } +void smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo) +{ + if(!allow_upstream_smgr_api) { + elog(FATAL, "An extension is trying to use the traditional smgrcreate method, while another loaded extension (pg_tde) requires the new API."); + } + + smgrcreate2(reln->smgr_rlocator.locator, reln, forknum, isRedo); +} + /* * smgrcreate() -- Create a new relation. * @@ -407,7 +418,7 @@ smgrexists(SMgrRelation reln, ForkNumber forknum) * to be created. */ void -smgrcreate(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo) +smgrcreate2(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo) { smgrsw[reln->smgr_which].smgr_create(relold, reln, forknum, isRedo); } diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index b148a7c2fd9a5..9a48e9138cb9e 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3876,7 +3876,7 @@ RelationSetNewRelfilenumber(Relation relation, char persistence) /* handle these directly, at least for now */ SMgrRelation srel; - srel = RelationCreateStorage(relation->rd_locator, newrlocator, persistence, true); + srel = RelationCreateStorage2(relation->rd_locator, newrlocator, persistence, true); smgrclose(srel); } else diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index b8b9e67e6dbee..6afb3d31a9730 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -68,6 +68,28 @@ extern void index_check_primary_key(Relation heapRel, #define INDEX_CREATE_INVALID (1 << 6) extern Oid index_create(Relation heapRelation, + const char *indexRelationName, + Oid indexRelationId, + Oid parentIndexRelid, + Oid parentConstraintId, + RelFileNumber relFileNumber, + IndexInfo *indexInfo, + const List *indexColNames, + Oid accessMethodId, + Oid tableSpaceId, + const Oid *collationIds, + const Oid *opclassIds, + const Datum *opclassOptions, + const int16 *coloptions, + const NullableDatum *stattargets, + Datum reloptions, + bits16 flags, + bits16 constr_flags, + bool allow_system_table_mods, + bool is_internal, + Oid *constraintId); + +extern Oid index_create2(Relation heapRelation, const char *indexRelationName, Oid indexRelationId, Oid parentIndexRelid, diff --git a/src/include/catalog/storage.h b/src/include/catalog/storage.h index f935c843457f7..2c995b008cf0d 100644 --- a/src/include/catalog/storage.h +++ b/src/include/catalog/storage.h @@ -22,7 +22,10 @@ /* GUC variables */ extern PGDLLIMPORT int wal_skip_threshold; -extern SMgrRelation RelationCreateStorage(RelFileLocator oldlocator, +extern SMgrRelation RelationCreateStorage(RelFileLocator rlocator, + char relpersistence, + bool register_delete); +extern SMgrRelation RelationCreateStorage2(RelFileLocator oldlocator, RelFileLocator rlocator, char relpersistence, bool register_delete); diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index 55fce3fa04ad7..9806e5e22ae5b 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -23,6 +23,7 @@ typedef uint8 SMgrId; #define MaxSMgrId UINT8_MAX extern PGDLLIMPORT SMgrId storage_manager_id; +extern bool allow_upstream_smgr_api; /* * smgr.c maintains a table of SMgrRelation objects, which are essentially @@ -130,7 +131,8 @@ extern void smgrdestroyall(void); extern void smgrrelease(SMgrRelation reln); extern void smgrreleaseall(void); extern void smgrreleaserellocator(RelFileLocatorBackend rlocator); -extern void smgrcreate(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo); +extern void smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo); +extern void smgrcreate2(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo); extern void smgrdosyncall(SMgrRelation *rels, int nrels); extern void smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo); extern void smgrextend(SMgrRelation reln, ForkNumber forknum,