Skip to content
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

#7782 - catch when Postgres planning removes all Citus tables #7907

Open
wants to merge 1 commit into
base: release-13.0
Choose a base branch
from

Conversation

colm-mchugh
Copy link
Contributor

@colm-mchugh colm-mchugh commented Feb 20, 2025

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 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.

Copy link

codecov bot commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.46%. Comparing base (c55bc8c) to head (030d05a).
Report is 21 commits behind head on release-13.0.

Additional details and impacted files
@@               Coverage Diff                @@
##           release-13.0    #7907      +/-   ##
================================================
- Coverage         89.48%   89.46%   -0.03%     
================================================
  Files               276      276              
  Lines             60063    59903     -160     
  Branches           7524     7514      -10     
================================================
- Hits              53747    53590     -157     
- Misses             4166     4168       +2     
+ Partials           2150     2145       -5     

@colm-mchugh colm-mchugh marked this pull request as draft February 20, 2025 14:52
@colm-mchugh colm-mchugh force-pushed the colm/issue_7782 branch 3 times, most recently from 7cf5873 to 63725bf Compare February 21, 2025 13:39
@colm-mchugh colm-mchugh marked this pull request as ready for review February 21, 2025 14:28
@naisila
Copy link
Member

naisila commented Feb 22, 2025

What an interesting failure! Thanks for this PR.

We modify the distributed_planner function very rarely. Hopefully I will review when I am back from OOO.

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
Copy link
Contributor Author

What an interesting failure! Thanks for this PR.

We modify the distributed_planner function very rarely. Hopefully I will review when I am back from OOO.

Sure; this seems to be the best (only?) way to deal with this planning quirk, but lmk what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A logic bug with distributed table A logic bug caused by a UPDATE statement with distributed table
2 participants