-
Notifications
You must be signed in to change notification settings - Fork 671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PG17 compatibility: Fix Test Failure in multi_alter_table_add_const #7733
base: naisila/pg17_support
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## naisila/pg17_support #7733 +/- ##
========================================================
- Coverage 89.61% 89.60% -0.01%
========================================================
Files 274 274
Lines 59689 59689
Branches 7446 7446
========================================================
- Hits 53490 53486 -4
- Misses 4069 4071 +2
- Partials 2130 2132 +2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we lack in documentation right now, let me introduce this concept here as it's a good PR for it.
For new things that PG allows, like an exclusion constraint on partitioned tables, first we make sure that it works as it is supposed to in Citus.
If it does, our general practice is to create a new file to test it, namely pg17.sql
where we add all such things. We will populate this file with other things as we go on with the support. (For this reason we have pg16.sql
pg15.sql
tests etc.) I will include this in the documentation for new PG support.
With that in mind, could you adjust this PR accordingly? I don't think we need to add a new output file for multi_alter_table_add_constraints
. Instead, we can create pg17.sql
file which will have pg17.out
and pg17_0.out
, and include this particular test case there. In this case, the pg17_0.out
file will have the error exclusion constraints are not supported on partitioned tables
.
8e51831
to
304400c
Compare
update update . move the tests update update update style
fa4854a
to
765d397
Compare
@naisila Can you review the solution? If it is acceptable, I will update the PR to prepare it for merging into the release branch. |
In earlier versions of PostgreSQL, exclusion constraints were not allowed on partitioned tables. This is why the error in your regression test (ERROR: exclusion constraints are not supported on partitioned tables) was raised in PostgreSQL 16. In PostgreSQL 17, exclusion constraints are now allowed on partitioned tables, which is why the error no longer appears when you attempt to add an exclusion constraint.
The constraint exclusion mechanism, described in the documentation, relies on CHECK constraints to decide which partitions or child tables need to be queried.
CHECK constraints