From 23e73abd37e726fb4bbb4cf344f6775c3403bf45 Mon Sep 17 00:00:00 2001 From: Sven Klemm Date: Sun, 7 Jul 2024 08:45:25 +0200 Subject: [PATCH] Allow building against PG17 With these changes TimescaleDB can be built against PG17. Doing so still requires -DEXPERIMENTAL=ON.. Co-authored-by: Aleksander Alekseev --- src/bgw/job.c | 2 +- src/chunk_index.c | 42 +++--- src/compat/compat.h | 134 +++++++++++++++++- src/copy.c | 2 +- src/import/ht_hypertable_modify.c | 19 ++- src/loader/bgw_message_queue.c | 6 +- src/nodes/chunk_dispatch/chunk_dispatch.c | 14 +- src/nodes/chunk_dispatch/chunk_insert_state.c | 2 +- src/nodes/hypertable_modify.c | 13 +- src/process_utility.c | 7 +- test/src/test_utils.c | 21 ++- tsl/src/compression/compression.c | 2 +- tsl/src/compression/compression_storage.c | 11 +- tsl/src/compression/wal_utils.h | 8 +- tsl/src/reorder.c | 4 + 15 files changed, 240 insertions(+), 47 deletions(-) diff --git a/src/bgw/job.c b/src/bgw/job.c index 481db7bd1..9c57d895f 100644 --- a/src/bgw/job.c +++ b/src/bgw/job.c @@ -701,7 +701,7 @@ get_job_lock_for_delete(int32 job_id) if (VirtualTransactionIdIsValid(*vxid)) { - proc = BackendIdGetProc(vxid->backendId); + proc = VirtualTransactionGetProcCompat(vxid); if (proc != NULL && proc->isBackgroundWorker) { /* Simply assuming that this pid corresponds to the background worker diff --git a/src/chunk_index.c b/src/chunk_index.c index 6cf4e4e78..d06c0e85e 100644 --- a/src/chunk_index.c +++ b/src/chunk_index.c @@ -277,26 +277,28 @@ ts_chunk_index_create_post_adjustment(int32 hypertable_id, Relation template_ind if (template_indexrel->rd_index->indisprimary) flags |= INDEX_CREATE_IS_PRIMARY; - chunk_indexrelid = index_create(chunkrel, - indexname, - InvalidOid, - InvalidOid, - InvalidOid, - InvalidOid, - indexinfo, - colnames, - template_indexrel->rd_rel->relam, - tablespace, - template_indexrel->rd_indcollation, - indclassoid->values, - template_indexrel->rd_indoption, - reloptions, - flags, - 0, /* constr_flags constant and 0 - * for now */ - false, /* allow system table mods */ - false, /* is internal */ - NULL); /* constraintId */ + chunk_indexrelid = index_create_compat(chunkrel, + indexname, + InvalidOid, + InvalidOid, + InvalidOid, + InvalidOid, + indexinfo, + colnames, + template_indexrel->rd_rel->relam, + tablespace, + template_indexrel->rd_indcollation, + indclassoid->values, + NULL, /* opclassOptions */ + template_indexrel->rd_indoption, + NULL, /* stattargets */ + reloptions, + flags, + 0, /* constr_flags constant and 0 + * for now */ + false, /* allow system table mods */ + false, /* is internal */ + NULL); /* constraintId */ ReleaseSysCache(tuple); diff --git a/src/compat/compat.h b/src/compat/compat.h index 8c2832808..e5266825d 100644 --- a/src/compat/compat.h +++ b/src/compat/compat.h @@ -28,6 +28,7 @@ #define is_supported_pg_version_14(version) ((version >= 140000) && (version < 150000)) #define is_supported_pg_version_15(version) ((version >= 150000) && (version < 160000)) #define is_supported_pg_version_16(version) ((version >= 160000) && (version < 170000)) +#define is_supported_pg_version_17(version) ((version >= 170000) && (version < 180000)) /* * PG16 support is a WIP and not complete yet. @@ -35,11 +36,12 @@ */ #define is_supported_pg_version(version) \ (is_supported_pg_version_14(version) || is_supported_pg_version_15(version) || \ - is_supported_pg_version_16(version)) + is_supported_pg_version_16(version) || is_supported_pg_version_17(version)) #define PG14 is_supported_pg_version_14(PG_VERSION_NUM) #define PG15 is_supported_pg_version_15(PG_VERSION_NUM) #define PG16 is_supported_pg_version_16(PG_VERSION_NUM) +#define PG17 is_supported_pg_version_17(PG_VERSION_NUM) #define PG14_LT (PG_VERSION_NUM < 140000) #define PG14_GE (PG_VERSION_NUM >= 140000) @@ -48,6 +50,7 @@ #define PG16_LT (PG_VERSION_NUM < 160000) #define PG16_GE (PG_VERSION_NUM >= 160000) #define PG17_LT (PG_VERSION_NUM < 170000) +#define PG17_GE (PG_VERSION_NUM >= 170000) #if !(is_supported_pg_version(PG_VERSION_NUM)) #error "Unsupported PostgreSQL version" @@ -787,4 +790,133 @@ RestrictSearchPath(void) 0, false); } + +/* This macro was renamed in PG17, see 414f6c0fb79a */ +#define WAIT_EVENT_MESSAGE_QUEUE_INTERNAL WAIT_EVENT_MQ_INTERNAL + +/* 'flush' argument was added in 173b56f1ef59 */ +#define LogLogicalMessageCompat(prefix, message, size, transactional, flush) \ + LogLogicalMessage(prefix, message, size, transactional) + +/* 'stmt' argument was added in f21848de2013 */ +#define reindex_relation_compat(stmt, relid, flags, params) reindex_relation(relid, flags, params) + +/* 'mergeActions' argument was added in 5f2e179bd31e */ +#define CheckValidResultRelCompat(resultRelInfo, operation, mergeActions) \ + CheckValidResultRel(resultRelInfo, operation) + +/* 'vacuum_is_relation_owner' was renamed to 'vacuum_is_permitted_for_relation' in ecb0fd33720f */ +#define vacuum_is_permitted_for_relation_compat(relid, reltuple, options) \ + vacuum_is_relation_owner(relid, reltuple, options) + +/* + * 'BackendIdGetProc' was renamed to 'ProcNumberGetProc' in 024c52111757. + * Also 'backendId' was renamed to 'procNumber' + */ +#define VirtualTransactionGetProcCompat(vxid) BackendIdGetProc((vxid)->backendId) + +/* + * 'opclassOptions' argument was added in 784162357130. + * Previously indexInfo->ii_OpclassOptions was used instead. + * On top of that 'stattargets' argument was added in 6a004f1be87d. + */ +#define index_create_compat(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) \ + index_create(heapRelation, \ + indexRelationName, \ + indexRelationId, \ + parentIndexRelid, \ + parentConstraintId, \ + relFileNumber, \ + indexInfo, \ + indexColNames, \ + accessMethodId, \ + tableSpaceId, \ + collationIds, \ + opclassIds, \ + coloptions, \ + reloptions, \ + flags, \ + constr_flags, \ + allow_system_table_mods, \ + is_internal, \ + constraintId) + +#else /* PG17_GE */ + +#define LogLogicalMessageCompat(prefix, message, size, transactional, flush) \ + LogLogicalMessage(prefix, message, size, transactional, flush) + +#define reindex_relation_compat(stmt, relid, flags, params) \ + reindex_relation(stmt, relid, flags, params) + +#define CheckValidResultRelCompat(resultRelInfo, operation, mergeActions) \ + CheckValidResultRel(resultRelInfo, operation, mergeActions) + +#define vacuum_is_permitted_for_relation_compat(relid, reltuple, options) \ + vacuum_is_permitted_for_relation(relid, reltuple, options) + +#define VirtualTransactionGetProcCompat(vxid) ProcNumberGetProc(vxid->procNumber) + +#define index_create_compat(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) \ + index_create(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) #endif diff --git a/src/copy.c b/src/copy.c index eb6174b73..a3df8b01f 100644 --- a/src/copy.c +++ b/src/copy.c @@ -786,7 +786,7 @@ copyfrom(CopyChunkState *ccstate, ParseState *pstate, Hypertable *ht, MemoryCont #endif ExecInitResultRelation(estate, resultRelInfo, 1); - CheckValidResultRel(resultRelInfo, CMD_INSERT); + CheckValidResultRelCompat(resultRelInfo, CMD_INSERT, NIL); ExecOpenIndices(resultRelInfo, false); diff --git a/src/import/ht_hypertable_modify.c b/src/import/ht_hypertable_modify.c index 1bc75a22e..b79c69cb2 100644 --- a/src/import/ht_hypertable_modify.c +++ b/src/import/ht_hypertable_modify.c @@ -382,9 +382,13 @@ ht_ExecMergeMatched(ModifyTableContext * context, ResultRelInfo * resultRelInfo, /* * If there are no WHEN MATCHED actions, we are done. */ +#if PG17_GE + if (resultRelInfo->ri_MergeActions[MERGE_WHEN_MATCHED] == NIL) + return true; +#else if (resultRelInfo->ri_matchedMergeAction == NIL) return true; - +#endif /* * Make tuple and any needed join variables available to ExecQual and * ExecProject. The target's existing tuple is installed in the @@ -412,8 +416,11 @@ lmerge_matched:; SnapshotAny, resultRelInfo->ri_oldTupleSlot)) elog(ERROR, "failed to fetch the target tuple"); - +#if PG17_GE + foreach(l, resultRelInfo->ri_MergeActions[MERGE_WHEN_MATCHED]) { +#else foreach(l, resultRelInfo->ri_matchedMergeAction) { +#endif MergeActionState *relaction = (MergeActionState *) lfirst(l); CmdType commandType = relaction->mas_action->commandType; List *recheckIndexes = NIL; @@ -762,9 +769,15 @@ ht_ExecMergeNotMatched(ModifyTableContext * context, ResultRelInfo * resultRelIn * * XXX does this mean that we can avoid creating copies of * actionStates on partitioned tables, for not-matched actions? + * + * XXX do we need an additional support of NOT MATCHED BY SOURCE + * for PG >= 17? See PostgreSQL commit 0294df2f1f84 */ +#if PG17_GE + actionStates = cds->rri->ri_MergeActions[MERGE_WHEN_NOT_MATCHED_BY_TARGET]; +#else actionStates = cds->rri->ri_notMatchedMergeAction; - +#endif /* * Make source tuple available to ExecQual and ExecProject. We don't * need the target tuple, since the WHEN quals and targetlist can't diff --git a/src/loader/bgw_message_queue.c b/src/loader/bgw_message_queue.c index 0b3e173bd..d26d4f01c 100644 --- a/src/loader/bgw_message_queue.c +++ b/src/loader/bgw_message_queue.c @@ -242,7 +242,7 @@ ts_shm_mq_wait_for_attach(MessageQueue *queue, shm_mq_handle *ack_queue_handle) WaitLatch(MyLatch, WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, BGW_MQ_WAIT_INTERVAL, - WAIT_EVENT_MQ_INTERNAL); + WAIT_EVENT_MESSAGE_QUEUE_INTERNAL); ResetLatch(MyLatch); CHECK_FOR_INTERRUPTS(); @@ -286,7 +286,7 @@ enqueue_message_wait_for_ack(MessageQueue *queue, BgwMessage *message, WaitLatch(MyLatch, WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, BGW_ACK_WAIT_INTERVAL, - WAIT_EVENT_MQ_INTERNAL); + WAIT_EVENT_MESSAGE_QUEUE_INTERNAL); ResetLatch(MyLatch); CHECK_FOR_INTERRUPTS(); } @@ -383,7 +383,7 @@ send_ack(dsm_segment *seg, bool success) WaitLatch(MyLatch, WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, BGW_ACK_WAIT_INTERVAL, - WAIT_EVENT_MQ_INTERNAL); + WAIT_EVENT_MESSAGE_QUEUE_INTERNAL); ResetLatch(MyLatch); CHECK_FOR_INTERRUPTS(); } diff --git a/src/nodes/chunk_dispatch/chunk_dispatch.c b/src/nodes/chunk_dispatch/chunk_dispatch.c index 154f04d4d..5d0e0e9d8 100644 --- a/src/nodes/chunk_dispatch/chunk_dispatch.c +++ b/src/nodes/chunk_dispatch/chunk_dispatch.c @@ -252,7 +252,10 @@ chunk_dispatch_plan_create(PlannerInfo *root, RelOptInfo *relopt, CustomPath *be cscan->custom_scan_tlist = tlist; cscan->scan.plan.targetlist = tlist; -#if PG15_GE + /* + * XXX mergeUseOuterJoin is gone in PG17, see 0294df2f1f84 + */ +#if ((PG15_GE) && (PG17_LT)) if (root->parse->mergeUseOuterJoin) { /* replace expressions of ROWID_VAR */ @@ -375,8 +378,17 @@ chunk_dispatch_exec(CustomScanState *node) } for (int i = 0; i < ht->space->num_dimensions; i++) { + /* + * XXX do we need an additional support of NOT MATCHED BY SOURCE + * for PG >= 17? See PostgreSQL commit 0294df2f1f84 + */ +#if PG17_GE + List *actionStates = dispatch->dispatch_state->mtstate->resultRelInfo + ->ri_MergeActions[MERGE_WHEN_NOT_MATCHED_BY_TARGET]; +#else List *actionStates = dispatch->dispatch_state->mtstate->resultRelInfo->ri_notMatchedMergeAction; +#endif ListCell *l; foreach (l, actionStates) { diff --git a/src/nodes/chunk_dispatch/chunk_insert_state.c b/src/nodes/chunk_dispatch/chunk_insert_state.c index 582a8f7da..d5fac44de 100644 --- a/src/nodes/chunk_dispatch/chunk_insert_state.c +++ b/src/nodes/chunk_dispatch/chunk_insert_state.c @@ -511,7 +511,7 @@ ts_chunk_insert_state_create(Oid chunk_relid, const ChunkDispatch *dispatch) MemoryContext old_mcxt = MemoryContextSwitchTo(cis_context); relinfo = create_chunk_result_relation_info(dispatch, rel); - CheckValidResultRel(relinfo, chunk_dispatch_get_cmd_type(dispatch)); + CheckValidResultRelCompat(relinfo, chunk_dispatch_get_cmd_type(dispatch), NIL); state = palloc0(sizeof(ChunkInsertState)); state->cds = dispatch->dispatch_state; diff --git a/src/nodes/hypertable_modify.c b/src/nodes/hypertable_modify.c index 7769e5002..d5552584c 100644 --- a/src/nodes/hypertable_modify.c +++ b/src/nodes/hypertable_modify.c @@ -691,8 +691,17 @@ ExecModifyTable(CustomScanState *cs_node, PlanState *pstate) if (TupIsNull(context.planSlot)) break; -#if PG15_GE - /* copy INSERT merge action list to result relation info of corresponding chunk */ + /* + * copy INSERT merge action list to result relation info of corresponding chunk + * + * XXX do we need an additional support of NOT MATCHED BY SOURCE + * for PG >= 17? See PostgreSQL commit 0294df2f1f84 + */ +#if PG17_GE + if (cds && cds->rri && operation == CMD_MERGE) + cds->rri->ri_MergeActions[MERGE_WHEN_NOT_MATCHED_BY_TARGET] = + resultRelInfo->ri_MergeActions[MERGE_WHEN_NOT_MATCHED_BY_TARGET]; +#elif PG15_GE if (cds && cds->rri && operation == CMD_MERGE) cds->rri->ri_notMatchedMergeAction = resultRelInfo->ri_notMatchedMergeAction; #endif diff --git a/src/process_utility.c b/src/process_utility.c index 575bd177b..23bf96eb6 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -721,9 +721,9 @@ ts_get_all_vacuum_rels(bool is_vacuumcmd) relid = classform->oid; /* check permissions of relation */ - if (!vacuum_is_relation_owner(relid, - classform, - is_vacuumcmd ? VACOPT_VACUUM : VACOPT_ANALYZE)) + if (!vacuum_is_permitted_for_relation_compat(relid, + classform, + is_vacuumcmd ? VACOPT_VACUUM : VACOPT_ANALYZE)) continue; /* @@ -3738,7 +3738,6 @@ process_altertable_end_subcmd(Hypertable *ht, Node *parsetree, ObjectAddress *ob case AT_ColumnDefault: case AT_CookedColumnDefault: case AT_SetNotNull: - case AT_CheckNotNull: case AT_DropNotNull: case AT_AddOf: case AT_DropOf: diff --git a/test/src/test_utils.c b/test/src/test_utils.c index 9e7ed6405..1c8c3ed81 100644 --- a/test/src/test_utils.c +++ b/test/src/test_utils.c @@ -92,13 +92,20 @@ transaction_row_counter(void) { static LocalTransactionId last_lxid = 0; static int rows_seen = 0; - - if (last_lxid != MyProc->lxid) +#if PG17_GE + if (last_lxid != MyProc->vxid.lxid) { /* Reset it for each new transaction for predictable results. */ + rows_seen = 0; + last_lxid = MyProc->vxid.lxid; + } +#else + if (last_lxid != MyProc->lxid) + { rows_seen = 0; last_lxid = MyProc->lxid; } +#endif return rows_seen++; } @@ -205,12 +212,20 @@ ts_debug_sleepy_function() static LocalTransactionId last_lxid = 0; static int rows_seen = 0; - if (last_lxid != MyProc->lxid) +#if PG17_GE + if (last_lxid != MyProc->vxid.lxid) { /* Reset it for each new transaction for predictable results. */ + rows_seen = 0; + last_lxid = MyProc->vxid.lxid; + } +#else + if (last_lxid != MyProc->lxid) + { rows_seen = 0; last_lxid = MyProc->lxid; } +#endif rows_seen++; diff --git a/tsl/src/compression/compression.c b/tsl/src/compression/compression.c index 5db42ada1..f020162b3 100644 --- a/tsl/src/compression/compression.c +++ b/tsl/src/compression/compression.c @@ -155,7 +155,7 @@ truncate_relation(Oid table_oid) ReindexParams params = { 0 }; ReindexParams *options = ¶ms; - reindex_relation(table_oid, REINDEX_REL_PROCESS_TOAST, options); + reindex_relation_compat(NULL, table_oid, REINDEX_REL_PROCESS_TOAST, options); rel = table_open(table_oid, AccessExclusiveLock); CommandCounterIncrement(); table_close(rel, NoLock); diff --git a/tsl/src/compression/compression_storage.c b/tsl/src/compression/compression_storage.c index 7d37a3ce5..225f9f5ca 100644 --- a/tsl/src/compression/compression_storage.c +++ b/tsl/src/compression/compression_storage.c @@ -178,7 +178,11 @@ set_statistics_on_compressed_chunk(Oid compressed_table_id) Relation table_rel = table_open(compressed_table_id, ShareUpdateExclusiveLock); Relation attrelation = table_open(AttributeRelationId, RowExclusiveLock); TupleDesc table_desc = RelationGetDescr(table_rel); +#if PG17_LT + /* see comments about PG17+ below */ Oid compressed_data_type = ts_custom_type_cache_get(CUSTOM_TYPE_COMPRESSED_DATA)->type_oid; +#endif + for (int i = 0; i < table_desc->natts; i++) { Form_pg_attribute attrtuple; @@ -200,16 +204,19 @@ set_statistics_on_compressed_chunk(Oid compressed_table_id) attrtuple = (Form_pg_attribute) GETSTRUCT(tuple); - /* the planner should never look at compressed column statistics because +#if PG17_LT + /* The planner should never look at compressed column statistics because * it will not understand them. Statistics on the other columns, * segmentbys and metadata, are very important, so we increase their * target. + * + * There are no 'attstattarget' and 'attstattarget' fields in PG17+. */ if (col_attr->atttypid == compressed_data_type) attrtuple->attstattarget = 0; else attrtuple->attstattarget = 1000; - +#endif CatalogTupleUpdate(attrelation, &tuple->t_self, tuple); InvokeObjectPostAlterHook(RelationRelationId, compressed_table_id, attrtuple->attnum); diff --git a/tsl/src/compression/wal_utils.h b/tsl/src/compression/wal_utils.h index 2cb7efed5..ff736bce4 100644 --- a/tsl/src/compression/wal_utils.h +++ b/tsl/src/compression/wal_utils.h @@ -34,7 +34,7 @@ write_logical_replication_msg_compression_start() { if (is_compression_wal_markers_enabled()) { - LogLogicalMessage(COMPRESSION_MARKER_START, "", 0, true); + LogLogicalMessageCompat(COMPRESSION_MARKER_START, "", 0, true, true); } } @@ -43,7 +43,7 @@ write_logical_replication_msg_compression_end() { if (is_compression_wal_markers_enabled()) { - LogLogicalMessage(COMPRESSION_MARKER_END, "", 0, true); + LogLogicalMessageCompat(COMPRESSION_MARKER_END, "", 0, true, true); } } @@ -52,7 +52,7 @@ write_logical_replication_msg_decompression_start() { if (is_compression_wal_markers_enabled()) { - LogLogicalMessage(DECOMPRESSION_MARKER_START, "", 0, true); + LogLogicalMessageCompat(DECOMPRESSION_MARKER_START, "", 0, true, true); } } @@ -61,6 +61,6 @@ write_logical_replication_msg_decompression_end() { if (is_compression_wal_markers_enabled()) { - LogLogicalMessage(DECOMPRESSION_MARKER_END, "", 0, true); + LogLogicalMessageCompat(DECOMPRESSION_MARKER_END, "", 0, true, true); } } diff --git a/tsl/src/reorder.c b/tsl/src/reorder.c index 13fd876d5..f9c2b4e3c 100644 --- a/tsl/src/reorder.c +++ b/tsl/src/reorder.c @@ -1139,6 +1139,10 @@ swap_relation_files(Oid r1, Oid r2, bool swap_toast_by_content, bool is_internal * itself, the smgr close on pg_class must happen after all accesses in * this function. */ + +#if PG17_LT + /* Not needed as of 21d9c3ee4ef7 in the upstream */ RelationCloseSmgrByOid(r1); RelationCloseSmgrByOid(r2); +#endif }