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

Fix #7684 : order "planner" tree #7689

Closed
wants to merge 1 commit into from
Closed

Conversation

c2main
Copy link
Contributor

@c2main c2main commented Sep 25, 2024

Similar to other bugs affected by citus usage of ruletuils: the provided tree as input is distinct from the tree expected in ruleutils in PostgreSQL.
As a consequence, it is required in some places to re-order the tree structure to make it compliant or "back" to the parser tree (before it's rewritten and reordered by PostgreSQL to optimize execution). Or to lookup the target list and ensure it's the one "we expect". Fox this bug, the get_rule_sortgroupclause() is used to check if target list entry resname is defined, and use it directly if it exists.

No benchmark where run, it's not expected to impact a lot.

To evaluate for PostgreSQL 17 support! (patched only existing ruleutils 14, 15, 16)

Similar to other bugs affected by citus usage of ruletuils: the provided
tree as input is distinct from the tree expected in ruleutils in
PostgreSQL.
As a consequence, it is required in some places to re-order the tree
structure to make it compliant or "back" to the parser tree (before it's
rewritten and reordered by PostgreSQL to optimize execution). Or to
lookup the target list and ensure it's the one "we expect".
Fox this bug, the `get_rule_sortgroupclause()` is used to check if
target list entry `resname` is defined, and use it directly if it
exists.

No benchmark where run, it's not expected to impact a lot.
@c2main
Copy link
Contributor Author

c2main commented Sep 25, 2024

It's more subtle than it appeared.

@c2main c2main closed this Sep 25, 2024
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.

1 participant