#7782 - catch when Postgres planning removes all Citus tables #7907
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-13.0 #7907 +/- ##
================================================
- Coverage 89.48% 89.47% -0.02%
================================================
Files 276 276
Lines 60063 60022 -41
Branches 7524 7520 -4
================================================
- Hits 53747 53704 -43
- Misses 4166 4169 +3
+ Partials 2150 2149 -1 |
colm-mchugh
force-pushed
the
colm/issue_7782
branch
from
278d9f3 to
1c472be
Compare
colm-mchugh
force-pushed
the
colm/issue_7782
branch
3 times, most recently
from
7cf5873 to
63725bf
Compare
|
What an interesting failure! Thanks for this PR. We modify the |
colm-mchugh
force-pushed
the
colm/issue_7782
branch
from
63725bf to
030d05a
Compare
Sure; this seems to be the best (only?) way to deal with this planning quirk, but lmk what you think. |
| Node *origQuals = origQuery->jointree->quals; | ||
| Node *plannedQuals = plannedQuery->jointree->quals; |
There was a problem hiding this comment.
Any way we can run into this while in a MERGE query? Given that in PG17 we have a separate mergeJoinClause?
There was a problem hiding this comment.
Possibly... if the target is a local table ... I will verify, thanks for noticing.
There was a problem hiding this comment.
So a PG17+ MERGE query was not covered by the fix - have updated the PR correspondingly.
| Query *origQuery, List *rangeTableList, | ||
| Query *plannedQuery) | ||
| { | ||
| if (isDistributedQuery) |
There was a problem hiding this comment.
Sorry, I am a bit confused.
Wasn't the original issue that we skipped distributed planning due to isDistributedQuery being false when it should have been true?
It looks like the original issue was the other way around, since you are checking if isDistributedQuery is true here ...
EDIT: I now understood the issue. I had to re-read the PR description carefully.
DESCRIPTION: fix a planning error caused by a redundant WHERE clause (of the form WHERE true OR <expression>) Fix a Citus planning glitch, where the WHERE clause of the query is of the form: ` WHERE true OR <expression with 1 or more citus tables> ` and this is the only place in the query referencing a citus table. Postgres' standard planner transforms the WHERE clause to: ` WHERE true ` So the query now has no citus tables, confusing the Citus planner as described in issues #7782 and #7783. The fix is to check, after Postgres standard planner, if the Query has been transformed as shown, and re-run the check of whether or not the query needs distributed planning.
colm-mchugh
force-pushed
the
colm/issue_7782
branch
from
d7c122b to
0b20a9b
Compare
DESCRIPTION: fix a planning error caused by a redundant WHERE clause
Fix a Citus planning glitch that occurs in a DML query when the WHERE clause of the query is of the form:
WHERE true OR <expression with 1 or more citus tables>and this is the only place in the query referencing a citus table. Postgres' standard planner transforms the WHERE clause to:
WHERE trueSo the query now has no citus tables, confusing the Citus planner as described in issues #7782 and #7783. The fix is to check, after Postgres standard planner, if the Query has been transformed as shown, and re-run the check of whether or not the query needs distributed planning.