Repository navigation
fix(sql): reject unsupported relation modifiers - #24758
Draft
geoffreyclaude wants to merge 2 commits into
Draft
geoffreyclaude wants to merge 2 commits into
geoffreyclaude wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #24758 +/- ##
==========================================
+ Coverage 81.48% 81.59% +0.10%
==========================================
Files 1122 1123 +1
Lines 404248 410952 +6704
Branches 404248 410952 +6704
==========================================
+ Hits 329390 335300 +5910
- Misses 55547 55905 +358
- Partials 19311 19747 +436 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
DataFusion's built-in SQL planner ignored some relation modifiers that sqlparser successfully parsed. For example,
TABLESAMPLEcould silently become an ordinary full table scan. This happened in vanilla DataFusion when noRelationPlannerwas registered, and also after every registered planner declined the relation.Queries should not succeed after silently changing their meaning. The built-in fallback should reject modifiers it cannot implement.
What changes are included in this PR?
When the built-in planner handles a relation, it now rejects unsupported modifiers on:
TABLESAMPLE, and index hints;TABLESAMPLE;WITH ORDINALITYandSETTINGS.Registered
RelationPlanners still run before the built-in fallback. An extension can continue to handle any modifier it supports; the new errors apply only when no extension plans the relation.The affected sqlparser structures are destructured exhaustively. If sqlparser adds another field, the compiler will require an explicit planning decision.
Are these changes tested?
Yes. The regression test covers every new rejection path both in vanilla DataFusion and after a pass-through extension planner. A separate example test confirms that an extension can still handle
TABLESAMPLE.Are there any user-facing changes?
Yes. Relation modifiers that the built-in planner previously accepted but ignored now return a clear unsupported-planning error. Queries without these modifiers are unchanged, as are queries whose modifiers are handled by an extension planner. There are no public API changes.