PG17 compatibility: Resolve compilation issues by naisila · Pull Request #7699 · citusdata/citus

Naisila Puka (naisila) · 2024-10-06T19:36:14Z

PG17 compatibility - Part 1

This PR, together with #7700, provides successful compilation against PG17.0.

Notes to reviewer:

Review commit by commit - it's much cleaner that way.
When merged, Enable configure and add pg17 build test commits will be reverted. They are part of the PR to make sure that the PG17 build is actually successful - see Build for PG17 in the PR checks. These changes will be merged to the release-13.0 branch later, when the PG17 support is fully added.

(Original PR against the main branch #7651)

codecov · 2024-10-06T19:43:00Z

Codecov Report

Attention: Patch coverage is 92.50000% with 3 lines in your changes missing coverage. Please review.

Please upload report for BASE (release-13.0@9d36433). Learn more about missing BASE report.

Additional details and impacted files

@@               Coverage Diff               @@
##             release-13.0    #7699   +/-   ##
===============================================
  Coverage                ?   89.65%           
===============================================
  Files                   ?      274           
  Lines                   ?    59584           
  Branches                ?     7436           
===============================================
  Hits                    ?    53418           
  Misses                  ?     4035           
  Partials                ?     2131

Jeff Mealo (jmealo) · 2024-10-08T18:57:51Z

I'm seeing a different issue compiling this branch on arm64/MacOS than I do on main against 16 (neither work, so this isn't a regression):

Against 17 (this branch):

Undefined symbols for architecture arm64:
  "_deparse_shard_query", referenced from:
      _GenerateTaskListWithColocatedIntermediateResults in repartition_executor.o
      _GenerateTaskListWithRedistributedResults in repartition_executor.o
      _DeparseTaskQuery in deparse_shard_query.o
      _CreateInsertSelectPlan in insert_select_planner.o
      _CacheLocalPlanForShardQuery in local_plan_cache.o
      _deparse_shard_query_test in deparse_shard_query.o
  "_generate_operator_name", referenced from:
      _GetFunctionDDLCommand in function.o
  "_generate_relation_name", referenced from:
      _PreprocessGrantStmt in grant.o
      _PreprocessAlterPolicyStmt in policy.o
      _pg_get_tableschemadef_string in citus_ruleutils.o
      _pg_get_tablecolumnoptionsdef_string in citus_ruleutils.o
      _pg_get_table_grants in citus_ruleutils.o
  "_getOwnedSequences_internal", referenced from:
      _ConvertTable in alter_table.o
      _ExtractDefaultColumnsAndOwnedSequences in sequence.o
  "_get_merged_argument_list", referenced from:
      _CallDistributedProcedureRemotely in call.o
  "_pg_get_query_def", referenced from:
      _BuildSelectStatementViaStdPlanner in combine_query_planner.o
      _DeparseTaskQuery in deparse_shard_query.o
      _CacheLocalPlanForShardQuery in local_plan_cache.o
      _CreatePhysicalDistributedPlan in multi_physical_planner.o
      _QueryPushdownSqlTaskList in multi_physical_planner.o
      _GenerateSubplansForSubqueriesAndCTEs in recursive_planning.o
      _RecursivelyPlanSubqueriesAndCTEs in recursive_planning.o
      ...
  "_pg_get_rule_expr", referenced from:
      _CallDistributedProcedureRemotely in call.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [citus.dylib] Error 1
make: *** [extension] Error 2

Main against 16

ld: warning: ignoring file '/opt/local/lib/libcurl.4.dylib': found architecture 'arm64', required architecture 'x86_64'
Undefined symbols for architecture x86_64:
  "_curl_easy_cleanup", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_easy_getinfo", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_easy_init", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_easy_perform", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_easy_setopt", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
      _CollectBasicUsageStatistics in statistics_collection.o
      _CollectBasicUsageStatistics in statistics_collection.o
      _CollectBasicUsageStatistics in statistics_collection.o
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_easy_strerror", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_global_cleanup", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_global_init", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_slist_append", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
      _CollectBasicUsageStatistics in statistics_collection.o
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_slist_free_all", referenced from:
      _CollectBasicUsageStatistics in statistics_collection.o
  "_curl_version_info", referenced from:
      _WarnIfSyncDNS in statistics_collection.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [citus.dylib] Error 1
make: *** [extension] Error 2

Onur Tirtir (onurctirtir) · 2024-10-15T08:01:46Z

+		if (es->memory)
+		{
+			MemoryContextSwitchTo(saved_ctx);
+			MemoryContextMemConsumed(planner_ctx, &mem_counters);


should we switch back to older context after L454

I followed the logic in the Postgres code here. Let me paste the relevant lines, where the context is not switched back. Do you think that is enough?
https://github.com/postgres/postgres/blob/REL_17_0/src/backend/commands/explain.c#L495-L512
https://github.com/postgres/postgres/blob/REL_17_0/src/backend/commands/prepare.c#L637-L642

Onur Tirtir (onurctirtir) · 2024-10-15T08:25:46Z

Note to self; Only remaining commit to review is cfba6ff.

update: done

Onur Tirtir (onurctirtir) · 2024-10-15T08:26:21Z

Btw, thank you for cleanly separating the work into commits, it made reviews so much easier!

Onur Tirtir (onurctirtir)

LGTM. rajeshkt78, would you like to review cfba6ff since it introduces some changes to cdc subsystem, or should we just merge this?

Emel Şimşek (emelsimsek) · 2024-10-15T10:43:11Z

Looks like PG 17 introduced a new ACL called ACL_MAINTAIN

PG commit:
ecb0fd33720fab91df1207e85704f382f55e1eb7
postgres/postgres@ecb0fd3

Our deparser does not handle that

citus/src/backend/distributed/deparser/citus_ruleutils.c

Line 1351 in f695971

elog(ERROR, "unrecognized aclright: %d", aclright);

I understand that this commit is for getting Citus compile against PG17.

Let's create an issue to track "ACL_MAINTAIN" should be supported.

Emel Şimşek (emelsimsek) · 2024-10-15T11:17:48Z

MergeAction can have 3 merge kinds (now enum) in PG17, write compat
Relevant PG commit:
0294df2f1f842dfb0eed79007b21016f486a3c6c
postgres/postgres@0294df2

New merge kinds should be handled in deparser and Merge logic in Citus.
- MATCHED BY SOURCE
- MATCHED_BY_TARGET

Let's track this as a github issue.

Emel Şimşek (emelsimsek) · 2024-10-15T12:47:51Z

-)
-
-#endif
+static inline CitusNode *


Should we delete two lines here:

citus/src/backend/distributed/utils/citus_nodefuncs.c

Line 56 in f695971

/* support for CitusNewNode() macro */

We still have CitusNewNode(size_t size, CitusNodeTag tag) function, so I guess not.

CitusNode *newCitusNodeMacroHolder; is not referenced anywhere. So I assumed it was related to getting around an issue with macro usage. Now since we have a function instead, this line may be redundant and confusing.

Just noticed that, you are right. It is redundant. Let me remove it.

This is prep work for successful compilation with PG17 PG17added foreach_ptr, foreach_int and foreach_oid macros Relevant PG commit 14dd0f27d7cd56ffae9ecdbe324965073d01a9ff postgres/postgres@14dd0f2 We already have these macros, but they are different with the PG17 ones because our macros take a DECLARED variable, whereas the PG16 macros declare a locally-scoped loop variable themselves. Hence I am renaming our macros to foreach_declared_ I am separating this into its own PR since it touches many files. The main compilation PR is #7699

Relevant PG commit d060e921ea5aa47b6265174c32e1128cebdbc3df postgres/postgres@d060e92

Relevant PG commit 041b96802efa33d2bc9456f2ad946976b92b5ae1 postgres/postgres@041b968

Relevant PG commit: 89e5ef7e21812916c9cf9fcf56e45f0f74034656 postgres/postgres@89e5ef7

Relevant PG commit: 08e6344fd6423210b339e92c069bb979ba4e7cd6 postgres/postgres@08e6344

Relevant PG commit: f696c0cd5f299f1b51e214efc55a22a782cc175d postgres/postgres@f696c0c

Relevant PG commit: de3600452b61d1bc3967e9e37e86db8956c8f577 postgres/postgres@de36004

…ctions for it

…ve been renamed

Naisila Puka (naisila) · 2024-10-16T19:05:20Z

Looks like PG 17 introduced a new ACL called ACL_MAINTAIN
...
I understand that this commit is for getting Citus compile against PG17.

MergeAction can have 3 merge kinds (now enum) in PG17, write compat

Emel Şimşek (@emelsimsek) you are right, it's not part of the compile PR. We might need a separate issue for new features in PG17 which will encapsulate these subtasks, like this one: #7138. For now, I added your comments in here #7708

Naisila Puka (naisila) · 2024-10-16T19:58:49Z

Jeff Mealo (@jmealo) Thanks for catching that, as it's not a regression caused by this PR I opened an issue to track this: #7707

This reverts commit fb4efeb.

This reverts commit 7497419.

PG17 compatibility - Part 2 #7699 was the first PG17 compatibility PR merged to main branch, which provided ONLY successful Citus compilation with PG17.0. This PR, consider it as Part 2, provides ruleutils changes for PG17. Ruleutils changes is the first thing we should merge, after successful build. It's the core for deparsing logic in Citus. # Question: How do we add ruleutils changes? - We add a new ruleutils file specific to PG17. - We keep track of the changes in Postgres's ruleutils file from here https://github.com/postgres/postgres/commits/REL_17_0/src/backend/utils/adt/ruleutils.c - Per each commit in that history that belongs only to 17.0, we add the relevant changes to static functions to our ruleutils file for PG17. It's like a manual commit copying. # Check the PR's commits for detailed steps https://github.com/citusdata/citus/pull/7725/commits

This is the final commit that adds PG17 compatibility with Citus's current capabilities. You can use Citus community, release-13.0 branch, with PG17.1. --------- Specifically, this commit: - Enables PG17 in the configure script. - Adds PG17 tests to CI using test images that have 17.1 - Fixes an upgrade test: see below for details In `citus_prepare_upgrade()`, don't drop any_value when upgrading from PG16+, because PG16+ has its own any_value function. Attempting to do so results in the error seen in [pg16-pg17 upgrade](https://github.com/citusdata/citus/actions/runs/11768444117/job/32778340003?pr=7661): ``` ERROR: cannot drop function any_value(anyelement) because it is required by the database system CONTEXT: SQL statement "DROP AGGREGATE IF EXISTS pg_catalog.any_value(anyelement)" ``` When 16 becomes the minimum supported Postgres version, the drop statements can be removed. --------- Several PG17 Compatibility commits have been merged before this final one. All these subtasks are done #7653 See the list below: Compilation PR: #7699 Ruleutils PR: #7725 Sister PR for tests: citusdata/the-process#159 Helpful smaller PRs: - #7714 - #7726 - #7731 - #7732 - #7733 - #7738 - #7745 - #7747 - #7748 - #7749 - #7752 - #7755 - #7757 - #7759 - #7760 - #7761 - #7762 - #7765 - #7766 - #7768 - #7769 - #7771 - #7774 - #7776 - #7780 - #7781 - #7785 - #7788 - #7793 - #7796 --------- Co-authored-by: Colm <colmmchugh@microsoft.com>

This is prep work for successful compilation with PG17 PG17added foreach_ptr, foreach_int and foreach_oid macros Relevant PG commit 14dd0f27d7cd56ffae9ecdbe324965073d01a9ff postgres/postgres@14dd0f2 We already have these macros, but they are different with the PG17 ones because our macros take a DECLARED variable, whereas the PG16 macros declare a locally-scoped loop variable themselves. Hence I am renaming our macros to foreach_declared_ I am separating this into its own PR since it touches many files. The main compilation PR is #7699

This PR provides successful compilation against PG17.0. - Remove ExecFreeExprContext call Relevant PG commit d060e921ea5aa47b6265174c32e1128cebdbc3df postgres/postgres@d060e92 - PG17 uses streaming IO in analyze, fix scan_analyze_next_block function Relevant PG commit 041b96802efa33d2bc9456f2ad946976b92b5ae1 postgres/postgres@041b968 - Define ObjectClass for PG17+ only since it's removed Relevant PG commit: 89e5ef7e21812916c9cf9fcf56e45f0f74034656 postgres/postgres@89e5ef7 - Remove ReorderBufferTupleBuf structure. Relevant PG commit: 08e6344fd6423210b339e92c069bb979ba4e7cd6 postgres/postgres@08e6344 - Define colliculocale and daticulocale since they have been renamed Relevant PG commit: f696c0cd5f299f1b51e214efc55a22a782cc175d postgres/postgres@f696c0c - makeStringConst defined in PG17 Relevant PG commit: de3600452b61d1bc3967e9e37e86db8956c8f577 postgres/postgres@de36004 - RangeVarCallbackOwnsTable was replaced by RangeVarCallbackMaintainsTable Relevant PG commit: ecb0fd33720fab91df1207e85704f382f55e1eb7 postgres/postgres@ecb0fd3 - attstattarget is nullable, define pg compatible functions for it Relevant PG commit: 4f622503d6de975ac87448aea5cea7de4bc140d5 postgres/postgres@4f62250 - stxstattarget is nullable in PG17, write compat functions for it Relevant PG commit: 012460ee93c304fbc7220e5b55d9d0577fc766ab postgres/postgres@012460e - Use ResourceOwner to track WaitEventSet in PG17 Relevant PG commit: 50c67c2019ab9ade8aa8768bfe604cd802fe8591 postgres/postgres@50c67c2 - getIdentitySequence now uses Relation instead of relation_id Relevant PG commit: 509199587df73f06eda898ae13284292f4ae573a postgres/postgres@5091995 - Remove no-op tuplestore_donestoring function Relevant PG commit: 75680c3d805e2323cd437ac567f0677fdfc7b680 postgres/postgres@75680c3 - MergeAction can have 3 merge kinds (now enum) in PG17, write compat Relevant PG commit: 0294df2f1f842dfb0eed79007b21016f486a3c6c postgres/postgres@0294df2 - EXPLAIN (MEMORY) is added, make changes to ExplainOnePlan Relevant PG commit: 5de890e3610d5a12cdaea36413d967cf5c544e20 postgres/postgres@5de890e - LIMIT_OPTION_DEFAULT has been removed as it's useless, use LIMIT_OPTION_COUNT Relevant PG commit: a6be0600ac3b71dda8277ab0fcbe59ee101ac1ce postgres/postgres@a6be060 - write compat for create_foreignscan_path bcs of more arguments in PG17 Relevant PG commit: 9e9931d2bf40e2fea447d779c2e133c2c1256ef3 postgres/postgres@9e9931d - pgprocno and lxid have been combined into a struct in PGPROC Relevant PG commits: 28f3915b73f75bd1b50ba070f56b34241fe53fd1 postgres/postgres@28f3915 ab355e3a88de745607f6dd4c21f0119b5c68f2ad postgres/postgres@ab355e3 024c521117579a6d356050ad3d78fdc95e44eefa postgres/postgres@024c521 - Simplify CitusNewNode (#7434) postgres refactored newNode() in PG 17, the main point for doing this is the original tricks is no longer neccessary for modern compilers[1]. This does the same for Citus. This should have no backward compatibility issues since it just replaces palloc0fast with palloc0. This is good for forward compatibility since palloc0fast no longer exists in PG 17. [1] https://www.postgresql.org/message-id/b51f1fa7-7e6a-4ecc-936d-90a8a1659e7c@iki.fi (cherry picked from commit 4b295cc)

PG17 compatibility - Part 2 #7699 was the first PG17 compatibility PR merged to main branch, which provided ONLY successful Citus compilation with PG17.0. This PR, consider it as Part 2, provides ruleutils changes for PG17. Ruleutils changes is the first thing we should merge, after successful build. It's the core for deparsing logic in Citus. # Question: How do we add ruleutils changes? - We add a new ruleutils file specific to PG17. - We keep track of the changes in Postgres's ruleutils file from here https://github.com/postgres/postgres/commits/REL_17_0/src/backend/utils/adt/ruleutils.c - Per each commit in that history that belongs only to 17.0, we add the relevant changes to static functions to our ruleutils file for PG17. It's like a manual commit copying. # Check the PR's commits for detailed steps https://github.com/citusdata/citus/pull/7725/commits

This is the final commit that adds PG17 compatibility with Citus's current capabilities. You can use Citus community, release-13.0 branch, with PG17.1. --------- Specifically, this commit: - Enables PG17 in the configure script. - Adds PG17 tests to CI using test images that have 17.1 - Fixes an upgrade test: see below for details In `citus_prepare_upgrade()`, don't drop any_value when upgrading from PG16+, because PG16+ has its own any_value function. Attempting to do so results in the error seen in [pg16-pg17 upgrade](https://github.com/citusdata/citus/actions/runs/11768444117/job/32778340003?pr=7661): ``` ERROR: cannot drop function any_value(anyelement) because it is required by the database system CONTEXT: SQL statement "DROP AGGREGATE IF EXISTS pg_catalog.any_value(anyelement)" ``` When 16 becomes the minimum supported Postgres version, the drop statements can be removed. --------- Several PG17 Compatibility commits have been merged before this final one. All these subtasks are done #7653 See the list below: Compilation PR: #7699 Ruleutils PR: #7725 Sister PR for tests: citusdata/the-process#159 Helpful smaller PRs: - #7714 - #7726 - #7731 - #7732 - #7733 - #7738 - #7745 - #7747 - #7748 - #7749 - #7752 - #7755 - #7757 - #7759 - #7760 - #7761 - #7762 - #7765 - #7766 - #7768 - #7769 - #7771 - #7774 - #7776 - #7780 - #7781 - #7785 - #7788 - #7793 - #7796 --------- Co-authored-by: Colm <colmmchugh@microsoft.com>

This is prep work for successful compilation with PG17 PG17added foreach_ptr, foreach_int and foreach_oid macros Relevant PG commit 14dd0f27d7cd56ffae9ecdbe324965073d01a9ff postgres/postgres@14dd0f2 We already have these macros, but they are different with the PG17 ones because our macros take a DECLARED variable, whereas the PG16 macros declare a locally-scoped loop variable themselves. Hence I am renaming our macros to foreach_declared_ I am separating this into its own PR since it touches many files. The main compilation PR is #7699

This PR provides successful compilation against PG17.0. - Remove ExecFreeExprContext call Relevant PG commit d060e921ea5aa47b6265174c32e1128cebdbc3df postgres/postgres@d060e92 - PG17 uses streaming IO in analyze, fix scan_analyze_next_block function Relevant PG commit 041b96802efa33d2bc9456f2ad946976b92b5ae1 postgres/postgres@041b968 - Define ObjectClass for PG17+ only since it's removed Relevant PG commit: 89e5ef7e21812916c9cf9fcf56e45f0f74034656 postgres/postgres@89e5ef7 - Remove ReorderBufferTupleBuf structure. Relevant PG commit: 08e6344fd6423210b339e92c069bb979ba4e7cd6 postgres/postgres@08e6344 - Define colliculocale and daticulocale since they have been renamed Relevant PG commit: f696c0cd5f299f1b51e214efc55a22a782cc175d postgres/postgres@f696c0c - makeStringConst defined in PG17 Relevant PG commit: de3600452b61d1bc3967e9e37e86db8956c8f577 postgres/postgres@de36004 - RangeVarCallbackOwnsTable was replaced by RangeVarCallbackMaintainsTable Relevant PG commit: ecb0fd33720fab91df1207e85704f382f55e1eb7 postgres/postgres@ecb0fd3 - attstattarget is nullable, define pg compatible functions for it Relevant PG commit: 4f622503d6de975ac87448aea5cea7de4bc140d5 postgres/postgres@4f62250 - stxstattarget is nullable in PG17, write compat functions for it Relevant PG commit: 012460ee93c304fbc7220e5b55d9d0577fc766ab postgres/postgres@012460e - Use ResourceOwner to track WaitEventSet in PG17 Relevant PG commit: 50c67c2019ab9ade8aa8768bfe604cd802fe8591 postgres/postgres@50c67c2 - getIdentitySequence now uses Relation instead of relation_id Relevant PG commit: 509199587df73f06eda898ae13284292f4ae573a postgres/postgres@5091995 - Remove no-op tuplestore_donestoring function Relevant PG commit: 75680c3d805e2323cd437ac567f0677fdfc7b680 postgres/postgres@75680c3 - MergeAction can have 3 merge kinds (now enum) in PG17, write compat Relevant PG commit: 0294df2f1f842dfb0eed79007b21016f486a3c6c postgres/postgres@0294df2 - EXPLAIN (MEMORY) is added, make changes to ExplainOnePlan Relevant PG commit: 5de890e3610d5a12cdaea36413d967cf5c544e20 postgres/postgres@5de890e - LIMIT_OPTION_DEFAULT has been removed as it's useless, use LIMIT_OPTION_COUNT Relevant PG commit: a6be0600ac3b71dda8277ab0fcbe59ee101ac1ce postgres/postgres@a6be060 - write compat for create_foreignscan_path bcs of more arguments in PG17 Relevant PG commit: 9e9931d2bf40e2fea447d779c2e133c2c1256ef3 postgres/postgres@9e9931d - pgprocno and lxid have been combined into a struct in PGPROC Relevant PG commits: 28f3915b73f75bd1b50ba070f56b34241fe53fd1 postgres/postgres@28f3915 ab355e3a88de745607f6dd4c21f0119b5c68f2ad postgres/postgres@ab355e3 024c521117579a6d356050ad3d78fdc95e44eefa postgres/postgres@024c521 - Simplify CitusNewNode (#7434) postgres refactored newNode() in PG 17, the main point for doing this is the original tricks is no longer neccessary for modern compilers[1]. This does the same for Citus. This should have no backward compatibility issues since it just replaces palloc0fast with palloc0. This is good for forward compatibility since palloc0fast no longer exists in PG 17. [1] https://www.postgresql.org/message-id/b51f1fa7-7e6a-4ecc-936d-90a8a1659e7c@iki.fi (cherry picked from commit 4b295cc)

PG17 compatibility - Part 2 #7699 was the first PG17 compatibility PR merged to main branch, which provided ONLY successful Citus compilation with PG17.0. This PR, consider it as Part 2, provides ruleutils changes for PG17. Ruleutils changes is the first thing we should merge, after successful build. It's the core for deparsing logic in Citus. # Question: How do we add ruleutils changes? - We add a new ruleutils file specific to PG17. - We keep track of the changes in Postgres's ruleutils file from here https://github.com/postgres/postgres/commits/REL_17_0/src/backend/utils/adt/ruleutils.c - Per each commit in that history that belongs only to 17.0, we add the relevant changes to static functions to our ruleutils file for PG17. It's like a manual commit copying. # Check the PR's commits for detailed steps https://github.com/citusdata/citus/pull/7725/commits

This is the final commit that adds PG17 compatibility with Citus's current capabilities. You can use Citus community, release-13.0 branch, with PG17.1. --------- Specifically, this commit: - Enables PG17 in the configure script. - Adds PG17 tests to CI using test images that have 17.1 - Fixes an upgrade test: see below for details In `citus_prepare_upgrade()`, don't drop any_value when upgrading from PG16+, because PG16+ has its own any_value function. Attempting to do so results in the error seen in [pg16-pg17 upgrade](https://github.com/citusdata/citus/actions/runs/11768444117/job/32778340003?pr=7661): ``` ERROR: cannot drop function any_value(anyelement) because it is required by the database system CONTEXT: SQL statement "DROP AGGREGATE IF EXISTS pg_catalog.any_value(anyelement)" ``` When 16 becomes the minimum supported Postgres version, the drop statements can be removed. --------- Several PG17 Compatibility commits have been merged before this final one. All these subtasks are done #7653 See the list below: Compilation PR: #7699 Ruleutils PR: #7725 Sister PR for tests: citusdata/the-process#159 Helpful smaller PRs: - #7714 - #7726 - #7731 - #7732 - #7733 - #7738 - #7745 - #7747 - #7748 - #7749 - #7752 - #7755 - #7757 - #7759 - #7760 - #7761 - #7762 - #7765 - #7766 - #7768 - #7769 - #7771 - #7774 - #7776 - #7780 - #7781 - #7785 - #7788 - #7793 - #7796 --------- Co-authored-by: Colm <colmmchugh@microsoft.com>

Naisila Puka (naisila) force-pushed the naisila/pg17_compile branch from a53a90f to 65d055e Compare October 6, 2024 19:38

Naisila Puka (naisila) force-pushed the naisila/pg17_compile branch from 65d055e to e71d521 Compare October 6, 2024 19:46

Naisila Puka (naisila) changed the base branch from release-13.0 to naisila/foreach_macros October 6, 2024 19:46

Naisila Puka (naisila) mentioned this pull request Oct 6, 2024

Rename foreach_ macros to foreach_declared_ macros #7700

Merged

Jeff Mealo (jmealo) mentioned this pull request Oct 8, 2024

PostgreSQL 17 support #7605

Closed

Onur Tirtir (onurctirtir) reviewed Oct 15, 2024

View reviewed changes

Onur Tirtir (onurctirtir) self-requested a review October 15, 2024 08:32

Onur Tirtir (onurctirtir) approved these changes Oct 15, 2024

View reviewed changes

Emel Şimşek (emelsimsek) requested changes Oct 15, 2024

View reviewed changes

Comment thread src/backend/distributed/deparser/citus_ruleutils.c Outdated

Comment thread src/backend/distributed/deparser/citus_ruleutils.c Outdated

Emel Şimşek (emelsimsek) reviewed Oct 15, 2024

View reviewed changes

Comment thread src/include/pg_version_compat.h

Emel Şimşek (emelsimsek) requested changes Oct 15, 2024

View reviewed changes

Comment thread src/include/pg_version_compat.h Outdated

Emel Şimşek (emelsimsek) reviewed Oct 15, 2024

View reviewed changes

Comment thread src/backend/distributed/planner/multi_explain.c

Emel Şimşek (emelsimsek) reviewed Oct 15, 2024

View reviewed changes

Comment thread src/backend/distributed/planner/multi_physical_planner.c

Emel Şimşek (emelsimsek) reviewed Oct 15, 2024

View reviewed changes

Comment thread .github/workflows/build_and_test.yml Outdated

Base automatically changed from naisila/foreach_macros to release-13.0 October 16, 2024 14:01

Naisila Puka (naisila) added 7 commits October 16, 2024 21:28

Enable configure

fb4efeb

Remove ExecFreeExprContext call

4685be7

Relevant PG commit d060e921ea5aa47b6265174c32e1128cebdbc3df postgres/postgres@d060e92

PG17 uses streaming IO in analyze, fix scan_analyze_next_block function

b4d36f6

Relevant PG commit 041b96802efa33d2bc9456f2ad946976b92b5ae1 postgres/postgres@041b968

Define ObjectClass for PG17+ only since it's removed

56f85a1

Relevant PG commit: 89e5ef7e21812916c9cf9fcf56e45f0f74034656 postgres/postgres@89e5ef7

Remove ReorderBufferTupleBuf structure.

f5c3b0a

Relevant PG commit: 08e6344fd6423210b339e92c069bb979ba4e7cd6 postgres/postgres@08e6344

Define colliculocale and daticulocale since they have been renamed

ad8eaf7

Relevant PG commit: f696c0cd5f299f1b51e214efc55a22a782cc175d postgres/postgres@f696c0c

makeStringConst defined in PG17

b81c8c8

Relevant PG commit: de3600452b61d1bc3967e9e37e86db8956c8f577 postgres/postgres@de36004

Naisila Puka (naisila) added 2 commits October 16, 2024 21:52

Address review on attstattarget is nullable, define pg compatible fun…

ef60a25

…ctions for it

Address review on Define colliculocale and daticulocale since they ha…

06727b6

…ve been renamed

Naisila Puka (naisila) mentioned this pull request Oct 16, 2024

PG17.1 Support - Regression tests sanity #7653

Closed

45 tasks

Address review use int instead of int16 and int32

cd76567

Naisila Puka (naisila) force-pushed the naisila/pg17_compile branch from e71d521 to cd76567 Compare October 16, 2024 19:54

Naisila Puka (naisila) mentioned this pull request Oct 16, 2024

Undefined symbols for architecture arm64 when compiling #7707

Open

Naisila Puka (naisila) requested a review from Emel Şimşek (emelsimsek) October 16, 2024 19:59

Naisila Puka (naisila) mentioned this pull request Oct 16, 2024

PG17 compatibility: Resolve compilation issues #7651

Closed

Emel Şimşek (emelsimsek) approved these changes Oct 17, 2024

View reviewed changes

Naisila Puka (naisila) added 3 commits October 17, 2024 14:29

Address reviews

980a425

Revert "Enable configure"

532cc95

This reverts commit fb4efeb.

Revert "add pg17 build test"

c3e0bc8

This reverts commit 7497419.

Naisila Puka (naisila) merged commit da2624c into release-13.0 Oct 17, 2024

Naisila Puka (naisila) deleted the naisila/pg17_compile branch October 17, 2024 12:37

This was referenced Nov 10, 2024

PG17 compatibility: ruleutils #7725

Merged

Adds PG17.1 support - Regression tests sanity #7661

Merged

Ivan Kush (ivan-v-kush) mentioned this pull request Jan 15, 2025

[Request]: Add PostgreSQL 17 Support to Hydra (Cloud Native Postgres Operator) hydradatabase/columnar#272

Open

Uh oh!

Conversation

Naisila Puka (naisila) commented Oct 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Jeff Mealo (jmealo) commented Oct 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Onur Tirtir (onurctirtir) Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

Naisila Puka (naisila) Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

Onur Tirtir (onurctirtir) commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Onur Tirtir (onurctirtir) commented Oct 15, 2024

Uh oh!

Onur Tirtir (onurctirtir) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emel Şimşek (emelsimsek) commented Oct 15, 2024

Uh oh!

Uh oh!

Emel Şimşek (emelsimsek) commented Oct 15, 2024

Uh oh!

Uh oh!

Uh oh!

Emel Şimşek (emelsimsek) Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

Naisila Puka (naisila) Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Emel Şimşek (emelsimsek) Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

Naisila Puka (naisila) Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Naisila Puka (naisila) commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Naisila Puka (naisila) commented Oct 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Naisila Puka (naisila) commented Oct 6, 2024 •

edited

Loading

codecov Bot commented Oct 6, 2024 •

edited

Loading

Jeff Mealo (jmealo) commented Oct 8, 2024 •

edited

Loading

Onur Tirtir (onurctirtir) commented Oct 15, 2024 •

edited

Loading

Naisila Puka (naisila) Oct 16, 2024 •

edited

Loading

Naisila Puka (naisila) commented Oct 16, 2024 •

edited

Loading