Avoid rewriting unsharded queries and split semantic analysis in two#15217
Merged
frouioui merged 12 commits intovitessio:mainfrom Feb 14, 2024
Merged
Avoid rewriting unsharded queries and split semantic analysis in two#15217frouioui merged 12 commits intovitessio:mainfrom
frouioui merged 12 commits intovitessio:mainfrom
Conversation
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Andres Taylor <andres@planetscale.com>
c7c7536 to
b51d049
Compare
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Contributor
|
Do we need to backport this to 18 and 19? |
Member
|
unit test are failing |
Signed-off-by: Andres Taylor <andres@planetscale.com>
harshit-gangal
approved these changes
Feb 14, 2024
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #15217 +/- ##
========================================
Coverage 67.43% 67.44%
========================================
Files 1560 1560
Lines 192789 192899 +110
========================================
+ Hits 130014 130099 +85
- Misses 62775 62800 +25 ☔ View full report in Codecov by Sentry. |
systay
approved these changes
Feb 14, 2024
7434e1f to
f44bfce
Compare
frouioui
added a commit
that referenced
this pull request
Feb 14, 2024
…15217) Signed-off-by: Andres Taylor <andres@planetscale.com> Co-authored-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Member
|
There is still an issue with the routing This is being sent to the wrong keyspace |
Collaborator
This hasn't changed with this PR. The same thing happens on |
systay
pushed a commit
that referenced
this pull request
Feb 14, 2024
…15217) Signed-off-by: Andres Taylor <andres@planetscale.com>
dbussink
pushed a commit
that referenced
this pull request
Feb 15, 2024
…nalysis in two (#15217) (#15230) Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com> Co-authored-by: Andres Taylor <andres@planetscale.com>
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.
Description
In a recent PR, we introduced a column alias expansion that has lead to a few users running into issues.
The idea with the column expansion was to make everything cleaner and simpler for the planner. In the cases where the planner was not involved (queries to a single unsharded keyspace), this however lead to problems.
With this PR, we change the semantic analysis and the early rewriter so that we do minimal work on the query when we don't need it.
In the early phase of the analyzer, we go over tables in FROM clauses, and see if any of them needs additional planning. If after this step we know that we don't need to plan the query, we are done and finish early, bypassing a lot of steps that are not necessary.
Related Issue(s)
PR that introduced the bug: #14935
Checklist
Deployment Notes