-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Planner Bug: Joins inside derived table #14974
Planner Bug: Joins inside derived table #14974
Conversation
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
|
4ed1291
to
1fed134
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #14974 +/- ##
===========================================
+ Coverage 47.29% 70.60% +23.30%
===========================================
Files 1137 1379 +242
Lines 238684 183009 -55675
===========================================
+ Hits 112895 129209 +16314
+ Misses 117168 53800 -63368
+ Partials 8621 0 -8621 ☔ View full report in Codecov by Sentry. |
a942f87
to
4a8887a
Compare
4a8887a
to
f60ed0a
Compare
f60ed0a
to
160bdcc
Compare
4df3f3a
to
008098d
Compare
proj := createProjection(ctx, src, "") | ||
proj.Columns = AliasedProjections(p.columns) | ||
if dt != nil { | ||
kopy := *dt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do not have a proper clone function for DerivedTable
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope. I'll add one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... I changed my mind. Don't think we need one. This is the only place we try to clone it, and it's not a pure cloning anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides that failing plan test and the comment I left, it looks good to me! That's a really good enhancement / bug fix, thank you 🙏🏻
Codecov is spanning a bunch of warnings in a lot of places in this PR, is this because of the cross-package coverage missing or is it due to actual missing plan tests?
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com> Signed-off-by: Andrés Taylor <andres@taylor.se>
Signed-off-by: Andres Taylor <andres@planetscale.com>
45b8e9e
to
4c8e0e3
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>
mcmp.Exec("INSERT INTO t1 (id1, id2) VALUES (1, 100), (2, 200), (3, 300), (4, 400), (5, 500);") | ||
mcmp.Exec("INSERT INTO t2 (id3, id4) VALUES (10, 1), (20, 2), (30, 3), (40, 4), (50, 99)") | ||
mcmp.Exec(`select t.a from (select t1.id2, t2.id3, (select id2 from t1 order by id2 limit 1) as a from t1 join t2 on t1.id1 = t2.id4) t`) | ||
mcmp.Exec(`SELECT COUNT(*) FROM (SELECT DISTINCT t1.id1 FROM t1 JOIN t2 ON t1.id1 = t2.id4) dt`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This last test will fail without the changes in this PR:
Error: Received unexpected error:
target: ks_union.-80.primary: vttablet: rpc error: code = NotFound desc = Unknown column 't2.id4' in 'field list' (errno 1054) (sqlstate 42S22) (CallerID: userData1): Sql: "select distinct t2.id4, 1, dt.`t2.id4` from (select t2.id4 as `t2.id4` from t2) as dt", BindVars: {}
target: ks_union.80-.primary: vttablet: rpc error: code = NotFound desc = Unknown column 't2.id4' in 'field list' (errno 1054) (sqlstate 42S22) (CallerID: userData1): Sql: "select distinct t2.id4, 1, dt.`t2.id4` from (select t2.id4 as `t2.id4` from t2) as dt", BindVars: {} (errno 1054) (sqlstate 42S22) during query: SELECT COUNT(*) FROM (SELECT DISTINCT t1.id1 FROM t1 JOIN t2 ON t1.id1 = t2.id4) dt
Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Signed-off-by: Andrés Taylor <andres@taylor.se> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com>
Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-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>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Description
In some situations, the vtgate planner was producing really bad plans around derived tables and columns.
In this PR I reworked a lot of the logic around derived tables and their columns.
Related Issue(s)
Fixes #14993
Checklist