From f993f4ff7dd81bd00450b607ff9322b5410356fe 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. --- 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 | 124 +++++++++++++++-------- src/backend/catalog/storage.c | 21 +++- src/backend/catalog/toasting.c | 16 +-- src/backend/commands/indexcmds.c | 18 ++-- 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 | 26 ++++- src/include/catalog/storage.h | 7 +- src/include/storage/smgr.h | 4 +- 15 files changed, 169 insertions(+), 86 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 320b5bd82a90b..cb97aa00bfaea 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -607,7 +607,7 @@ heapam_relation_set_new_filelocator(Relation rel, */ *minmulti = GetOldestMultiXactId(); - srel = RelationCreateStorage(oldlocator, *newrlocator, persistence, true); + srel = RelationCreateStoragePercona(oldlocator, *newrlocator, persistence, true); /* * If required, set up an init fork for an unlogged table so that it can @@ -617,7 +617,7 @@ heapam_relation_set_new_filelocator(Relation rel, { Assert(rel->rd_rel->relkind == RELKIND_RELATION || rel->rd_rel->relkind == RELKIND_TOASTVALUE); - smgrcreate(oldlocator, srel, INIT_FORKNUM, false); + smgrcreate_percona(oldlocator, srel, INIT_FORKNUM, false); log_smgrcreate(newrlocator, INIT_FORKNUM); } @@ -650,7 +650,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 = RelationCreateStoragePercona(rel->rd_locator, *newrlocator, rel->rd_rel->relpersistence, true); /* copy main fork */ RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, @@ -662,7 +662,7 @@ heapam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) { if (smgrexists(RelationGetSmgr(rel), forkNum)) { - smgrcreate(rel->rd_locator, dstrel, forkNum, false); + smgrcreate_percona(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 2179d2f73fa9a..3c5c9d281685f 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -487,7 +487,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); + smgrcreate_percona(rlocator, smgr, forknum, true); lastblock = smgrnblocks(smgr, forknum); diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 504097d8b3c0d..6b241819ad778 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -401,7 +401,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); + RelationCreateStoragePercona(prev_rlocator, new_rlocator, relpersistence, true); else Assert(false); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index f95c8e7ed6c09..f7bef3cc97e27 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -674,6 +674,39 @@ UpdateIndexRelation(Oid indexoid, heap_freetuple(tuple); } +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 (!percona_allow_upstream_smgr_api) + elog(ERROR, "An extension is trying to use the traditional index_create method, while another loaded extension (pg_tde) requires the new API."); + + return index_create_percona(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 @@ -723,28 +756,28 @@ UpdateIndexRelation(Oid indexoid, * Returns the OID of the created index. */ 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, - RelFileLocator *old_rlocator) +index_create_percona(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, + RelFileLocator *old_rlocator) { Oid heapRelationId = RelationGetRelid(heapRelation); Relation pg_class; @@ -1444,28 +1477,29 @@ 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, - newName, - InvalidOid, /* indexRelationId */ - InvalidOid, /* parentIndexRelid */ - InvalidOid, /* parentConstraintId */ - InvalidRelFileNumber, /* relFileNumber */ - newInfo, - indexColNames, - indexRelation->rd_rel->relam, - tablespaceOid, - indexRelation->rd_indcollation, - indclass->values, - opclassOptions, - indcoloptions->values, - stattargets, - reloptionsDatum, - INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT, - 0, - true, /* allow table to be a system catalog? */ - false, /* is_internal? */ - NULL, - &indexRelation->rd_locator); + newIndexId = index_create_percona(heapRelation, + newName, + InvalidOid, /* indexRelationId */ + InvalidOid, /* parentIndexRelid */ + InvalidOid, /* parentConstraintId */ + InvalidRelFileNumber, /* relFileNumber */ + newInfo, + indexColNames, + indexRelation->rd_rel->relam, + tablespaceOid, + indexRelation->rd_indcollation, + indclass->values, + opclassOptions, + indcoloptions->values, + stattargets, + reloptionsDatum, + INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT, + 0, + true, /* allow table to be a system + * catalog? */ + false, /* is_internal? */ + NULL, + &indexRelation->rd_locator); /* Close the relations used and clean up */ index_close(indexRelation, NoLock); @@ -3092,7 +3126,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); + smgrcreate_percona(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 c7136dfbd1a92..34801d3941039 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -105,6 +105,17 @@ AddPendingSync(const RelFileLocator *rlocator) pending->is_truncated = false; } +SMgrRelation +RelationCreateStorage(RelFileLocator rlocator, + char relpersistence, + bool register_delete) +{ + if (!percona_allow_upstream_smgr_api) + elog(ERROR, "An extension is trying to use the traditional RelationCreateStorage method, while another loaded extension (pg_tde) requires the new API."); + + return RelationCreateStoragePercona(rlocator, rlocator, relpersistence, register_delete); +} + /* * RelationCreateStorage * Create physical storage for a relation. @@ -119,8 +130,8 @@ AddPendingSync(const RelFileLocator *rlocator) * pass register_delete = false. */ SMgrRelation -RelationCreateStorage(RelFileLocator oldlocator, RelFileLocator rlocator, char relpersistence, - bool register_delete) +RelationCreateStoragePercona(RelFileLocator oldlocator, RelFileLocator rlocator, char relpersistence, + bool register_delete) { SMgrRelation srel; ProcNumber procNumber; @@ -148,7 +159,7 @@ RelationCreateStorage(RelFileLocator oldlocator, RelFileLocator rlocator, char r } srel = smgropen(rlocator, procNumber); - smgrcreate(oldlocator, srel, MAIN_FORKNUM, false); + smgrcreate_percona(oldlocator, srel, MAIN_FORKNUM, false); if (needs_wal) log_smgrcreate(&srel->smgr_rlocator.locator, MAIN_FORKNUM); @@ -992,7 +1003,7 @@ smgr_redo(XLogReaderState *record) SMgrRelation reln; reln = smgropen(xlrec->rlocator, INVALID_PROC_NUMBER); - smgrcreate(xlrec->rlocator, reln, xlrec->forkNum, true); + smgrcreate_percona(xlrec->rlocator, reln, xlrec->forkNum, true); } else if (info == XLOG_SMGR_TRUNCATE) { @@ -1013,7 +1024,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); + smgrcreate_percona(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 6e65d48ac8ba2..999f7b962ac6f 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -318,14 +318,14 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, coloptions[0] = 0; coloptions[1] = 0; - index_create(toast_rel, toast_idxname, toastIndexOid, InvalidOid, - InvalidOid, InvalidOid, - indexInfo, - list_make2("chunk_id", "chunk_seq"), - BTREE_AM_OID, - rel->rd_rel->reltablespace, - collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0, - INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL, NULL); + index_create_percona(toast_rel, toast_idxname, toastIndexOid, InvalidOid, + InvalidOid, InvalidOid, + indexInfo, + list_make2("chunk_id", "chunk_seq"), + BTREE_AM_OID, + rel->rd_rel->reltablespace, + collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0, + INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL, NULL); table_close(toast_rel, NoLock); diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index dffa67a8ddb90..80858bab32cd5 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1245,15 +1245,15 @@ DefineIndex(Oid tableId, constr_flags |= INDEX_CONSTR_CREATE_WITHOUT_OVERLAPS; indexRelationId = - index_create(rel, indexRelationName, indexRelationId, parentIndexId, - parentConstraintId, - stmt->oldNumber, indexInfo, indexColNames, - accessMethodId, tablespaceId, - collationIds, opclassIds, opclassOptions, - coloptions, NULL, reloptions, - flags, constr_flags, - allowSystemTableMods, !check_rights, - &createdConstraintId, NULL); + index_create_percona(rel, indexRelationName, indexRelationId, parentIndexId, + parentConstraintId, + stmt->oldNumber, indexInfo, indexColNames, + accessMethodId, tablespaceId, + collationIds, opclassIds, opclassOptions, + coloptions, NULL, reloptions, + flags, constr_flags, + allowSystemTableMods, !check_rights, + &createdConstraintId, NULL); ObjectAddressSet(address, RelationRelationId, indexRelationId); diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 098b58123e112..50fa774e1f62d 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); + smgrcreate_percona(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 8249b1b355ed1..97a5b50bdd253 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -17165,7 +17165,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 = RelationCreateStoragePercona(rel->rd_locator, newrlocator, rel->rd_rel->relpersistence, true); /* copy main fork */ RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, @@ -17177,7 +17177,7 @@ index_copy_data(Relation rel, RelFileLocator newrlocator) { if (smgrexists(RelationGetSmgr(rel), forkNum)) { - smgrcreate(rel->rd_locator, dstrel, forkNum, false); + smgrcreate_percona(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 15e42684907fc..9afe1b1e61f21 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -955,7 +955,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); + smgrcreate_percona(bmr.rel->rd_locator, bmr.smgr, fork, flags & EB_PERFORMING_RECOVERY); UnlockRelationForExtension(bmr.rel, ExclusiveLock); } @@ -5257,7 +5257,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); + RelationCreateStoragePercona(src_rlocator, dst_rlocator, relpersistence, false); /* copy main fork. */ RelationCopyStorageUsingBuffer(src_rlocator, dst_rlocator, MAIN_FORKNUM, @@ -5270,7 +5270,7 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator, if (smgrexists(src_rel, forkNum)) { /* TODO: for sure? */ - smgrcreate(src_rel->smgr_rlocator.locator, dst_rel, forkNum, false); + smgrcreate_percona(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 3ff378d50e4da..4a97819773ebe 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -84,6 +84,8 @@ static Size LargestSMgrRelationSize = 0; SMgrId storage_manager_id; +bool percona_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. @@ -459,6 +461,15 @@ smgrexists(SMgrRelation reln, ForkNumber forknum) return ret; } +void +smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo) +{ + if (!percona_allow_upstream_smgr_api) + elog(ERROR, "An extension is trying to use the traditional smgrcreate method, while another loaded extension (pg_tde) requires the new API."); + + smgrcreate_percona(reln->smgr_rlocator.locator, reln, forknum, isRedo); +} + /* * smgrcreate() -- Create a new relation. * @@ -467,7 +478,7 @@ smgrexists(SMgrRelation reln, ForkNumber forknum) * to be created. */ void -smgrcreate(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo) +smgrcreate_percona(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo) { HOLD_INTERRUPTS(); 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 25e267a2080e3..aabedf759bfb5 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -3880,7 +3880,7 @@ RelationSetNewRelfilenumber(Relation relation, char persistence) /* handle these directly, at least for now */ SMgrRelation srel; - srel = RelationCreateStorage(relation->rd_locator, newrlocator, persistence, true); + srel = RelationCreateStoragePercona(relation->rd_locator, newrlocator, persistence, true); smgrclose(srel); } else diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index 1af82f7d3591a..f0d4b40f2a819 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -87,8 +87,30 @@ extern Oid index_create(Relation heapRelation, bits16 constr_flags, bool allow_system_table_mods, bool is_internal, - Oid *constraintId, - RelFileLocator *old_rlocator); + Oid *constraintId); + +extern Oid index_create_percona(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, + RelFileLocator *old_rlocator); #define INDEX_CONSTR_CREATE_MARK_AS_PRIMARY (1 << 0) #define INDEX_CONSTR_CREATE_DEFERRABLE (1 << 1) diff --git a/src/include/catalog/storage.h b/src/include/catalog/storage.h index ecc3b792f4f54..b7d3eb0fa7a50 100644 --- a/src/include/catalog/storage.h +++ b/src/include/catalog/storage.h @@ -22,10 +22,13 @@ /* GUC variables */ extern PGDLLIMPORT int wal_skip_threshold; -extern SMgrRelation RelationCreateStorage(RelFileLocator oldlocator, - RelFileLocator rlocator, +extern SMgrRelation RelationCreateStorage(RelFileLocator rlocator, char relpersistence, bool register_delete); +extern SMgrRelation RelationCreateStoragePercona(RelFileLocator oldlocator, + RelFileLocator rlocator, + char relpersistence, + bool register_delete); extern void RelationDropStorage(Relation rel); extern void RelationPreserveStorage(RelFileLocator rlocator, bool atCommit); extern void RelationPreTruncate(Relation rel); diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index 08555ce5def5c..14b1bb94ae4aa 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -22,6 +22,7 @@ typedef uint8 SMgrId; extern PGDLLIMPORT SMgrId storage_manager_id; +extern bool percona_allow_upstream_smgr_api; /* * smgr.c maintains a table of SMgrRelation objects, which are essentially @@ -136,7 +137,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 smgrcreate_percona(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,