-
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 dml_recursive #7727
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 #7727 +/- ##
=====================================================
Coverage 89.61% 89.61%
=====================================================
Files 274 274
Lines 59689 59689
Branches 7446 7446
=====================================================
+ Hits 53490 53492 +2
+ Misses 4069 4067 -2
Partials 2130 2130 |
tenant_id | dept | info | tenant_id | ||
--------------------------------------------------------------------- | ||
14 | 3 | {"f1": 14, "f2": 196} | 14 | ||
23 | 5 | {"f1": 23, "f2": 529} | 23 | ||
24 | 6 | {"f1": 24, "f2": 576} | 24 | ||
3 | 0 | {"f1": 3, "f2": 9} | 3 | ||
33 | 8 | {"f1": 33, "f2": 1089} | 33 | ||
34 | 8 | {"f1": 34, "f2": 1156} | 34 | ||
4 | 1 | {"f1": 4, "f2": 16} | 4 | ||
43 | 10 | {"f1": 43, "f2": 1849} | 43 | ||
44 | 11 | {"f1": 44, "f2": 1936} | 44 | ||
53 | 13 | {"f1": 53, "f2": 2809} | 53 | ||
54 | 13 | {"f1": 54, "f2": 2916} | 54 | ||
63 | 15 | {"f1": 63, "f2": 3969} | 63 | ||
64 | 16 | {"f1": 64, "f2": 4096} | 64 | ||
73 | 18 | {"f1": 73, "f2": 5329} | 73 | ||
74 | 18 | {"f1": 74, "f2": 5476} | 74 | ||
83 | 20 | {"f1": 83, "f2": 6889} | 83 | ||
84 | 21 | {"f1": 84, "f2": 7056} | 84 | ||
93 | 23 | {"f1": 93, "f2": 8649} | 93 | ||
94 | 23 | {"f1": 94, "f2": 8836} | 94 | ||
(19 rows) | ||
|
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.
I think we need to take a closer look into the new query plan to see how that optimization allowed Citus to run the query now, you can ping me tomorrow for us to quickly debug this together.
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.
Nice fix, thanks @m3hm3t and @onurctirtir
Always appreciate a fix that avoids a new output file :)
Let's merge directly to release-13.0
and then rebase naisila/pg17_support
branch
EDIT: sorry, wrong PR
-- dml_recursive_0.out for PG16 and before | ||
-- dml_recursive.out for PG17 | ||
-- related commit | ||
-- PostgreSQL 17 includes an enhancement that allows the optimizer to transform correlated IN subqueries into more efficient join operations. |
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.
Let's hold off a bit more before merging this, until we are sure that nothing needs to change in the Citus codebase.
PostgreSQL 17 includes an enhancement that allows the optimizer to transform correlated IN subqueries into more efficient join operations.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9f1337639