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.

Sign up for GitHub

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

don't push Filter below Limit #1692

Merged
merged 17 commits into from
Apr 6, 2023
Merged

don't push Filter below Limit #1692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b85d5a5
fixed with different bugs
Mar 29, 2023
08a85f1
Merge branch 'main' into james/limit
Apr 3, 2023
f2812fd
aaaaa
Apr 3, 2023
0c0b33c
aaaaaa
Apr 4, 2023
67789b5
Merge branch 'main' into james/limit
Apr 4, 2023
2ff7ce0
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 4, 2023
e7121d0
i hope this is the solution
Apr 5, 2023
c28f24b
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 5, 2023
c685c5f
Merge branch 'main' into james/limit
Apr 5, 2023
33b7e42
tests and cleanup
Apr 5, 2023
a356f5f
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 5, 2023
45a3f91
remove unused code paths
Apr 5, 2023
937f5d6
remove newlines
Apr 5, 2023
865bf32
removing unused code
Apr 5, 2023
c2ba9b8
simplifying check
Apr 5, 2023
f43ead4
adding join tests
Apr 6, 2023
63ececa
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions enginetest/queries/queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2723,10 +2723,58 @@ var QueryTests = []QueryTest{
Query: "SELECT i FROM mytable ORDER BY i LIMIT 1 OFFSET 1;",
Expected: []sql.Row{{int64(2)}},
},
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like good subquery and single relation coverage! might be worth adding join tests, root and subscopes, might behave differently

Query: "SELECT i FROM (SELECT i FROM mytable LIMIT 1) sq WHERE i = 3;",
Expected: []sql.Row{},
},
{
Query: "SELECT i FROM (SELECT i FROM (SELECT i FROM mytable LIMIT 1) sq1) sq2 WHERE i = 3;",
Expected: []sql.Row{},
},
{
Query: "SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC LIMIT 1) sq WHERE i = 3;",
Expected: []sql.Row{{3}},
},
{
Query: "SELECT i FROM (SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC LIMIT 1) sq1) sq2 WHERE i = 3;",
Expected: []sql.Row{{3}},
},
{
Query: "SELECT i FROM (SELECT i FROM mytable WHERE i > 1) sq LIMIT 1;",
Expected: []sql.Row{{2}},
},
{
Query: "SELECT i FROM (SELECT i FROM (SELECT i FROM mytable WHERE i > 1) sq1) sq2 LIMIT 1;",
Expected: []sql.Row{{2}},
},
{
Query: "SELECT i FROM (SELECT i FROM (SELECT i FROM mytable) sq1 WHERE i > 1) sq2 LIMIT 1;",
Expected: []sql.Row{{2}},
},
{
Query: "SELECT i FROM (SELECT i FROM (SELECT i FROM mytable LIMIT 1) sq1 WHERE i > 1) sq2 LIMIT 10;",
Expected: []sql.Row{},
},
{
Query: "SELECT i FROM mytable WHERE i NOT IN (SELECT i FROM (SELECT * FROM (SELECT i as i, s as s FROM mytable) f) s)",
Expected: []sql.Row{},
},
{
Query: "SELECT * FROM (SELECT a.pk, b.i FROM one_pk a JOIN mytable b ORDER BY a.pk ASC, b.i ASC LIMIT 1) sq WHERE i != 0",
Expected: []sql.Row{
{0, 1},
},
},
{
Query: "SELECT * FROM (SELECT a.pk, b.i FROM one_pk a JOIN mytable b ORDER BY a.pk DESC, b.i DESC LIMIT 1) sq WHERE i != 0",
Expected: []sql.Row{
{3, 3},
},
},
{
Query: "SELECT * FROM (SELECT pk FROM one_pk WHERE pk < 2 LIMIT 1) a JOIN (SELECT i FROM mytable WHERE i > 1 LIMIT 1) b WHERE pk >= 2;",
Expected: []sql.Row{},
},
{
Query: "SELECT i FROM (SELECT 1 AS i FROM DUAL UNION SELECT 2 AS i FROM DUAL) some_is WHERE i NOT IN (SELECT i FROM (SELECT 1 as i FROM DUAL) different_is);",
Expected: []sql.Row{{int64(2)}},
Expand Down
234 changes: 227 additions & 7 deletions enginetest/queries/query_plans.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ inner join pq on true
"",
},
{
Query: `select row_number() over (order by i desc), mytable.i as i2
Query: `select row_number() over (order by i desc), mytable.i as i2
from mytable join othertable on i = i2 order by 1`,
ExpectedPlan: "Sort(row_number() over (order by i desc):0!null ASC nullsFirst)\n" +
" └─ Project\n" +
Expand Down Expand Up @@ -1438,7 +1438,7 @@ inner join pq on true
"",
},
{
Query: `select row_number() over (order by i desc), mytable.i as i2
Query: `select row_number() over (order by i desc), mytable.i as i2
from mytable join othertable on i = i2
where mytable.i = 2
order by 1`,
Expand Down Expand Up @@ -3982,8 +3982,8 @@ inner join pq on true
"",
},
{
Query: `SELECT pk,tpk.pk1,tpk2.pk1,tpk.pk2,tpk2.pk2 FROM one_pk
JOIN two_pk tpk ON pk=tpk.pk1 AND pk-1=tpk.pk2
Query: `SELECT pk,tpk.pk1,tpk2.pk1,tpk.pk2,tpk2.pk2 FROM one_pk
JOIN two_pk tpk ON pk=tpk.pk1 AND pk-1=tpk.pk2
JOIN two_pk tpk2 ON pk-1=TPK2.pk1 AND pk=tpk2.pk2
ORDER BY 1`,
ExpectedPlan: "Sort(one_pk.pk:0!null ASC nullsFirst)\n" +
Expand Down Expand Up @@ -4128,7 +4128,7 @@ inner join pq on true
"",
},
{
Query: `SELECT pk FROM one_pk
Query: `SELECT pk FROM one_pk
RIGHT JOIN two_pk tpk ON one_pk.pk=tpk.pk1 AND one_pk.pk=tpk.pk2
RIGHT JOIN two_pk tpk2 ON tpk.pk1=TPk2.pk2 AND tpk.pk2=TPK2.pk1`,
ExpectedPlan: "Project\n" +
Expand Down Expand Up @@ -4236,7 +4236,7 @@ inner join pq on true
"",
},
{
Query: `SELECT pk,nt.i,nt2.i FROM one_pk
Query: `SELECT pk,nt.i,nt2.i FROM one_pk
RIGHT JOIN niltable nt ON pk=nt.i
RIGHT JOIN niltable nt2 ON pk=nt2.i + 1`,
ExpectedPlan: "Project\n" +
Expand Down Expand Up @@ -7243,7 +7243,7 @@ With c as (
Where t1.I in (2,3)
) e
On b.I = e.i
) d
) d
) select * from c;`,
ExpectedPlan: "SubqueryAlias\n" +
" ├─ name: c\n" +
Expand Down Expand Up @@ -7303,6 +7303,226 @@ With c as (
" └─ columns: [i s]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM mytable LIMIT 1) sq WHERE i = 3;`,
ExpectedPlan: "SubqueryAlias\n" +
" ├─ name: sq\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Filter\n" +
" ├─ Eq\n" +
" │ ├─ mytable.i:0!null\n" +
" │ └─ 3 (tinyint)\n" +
" └─ Limit(1)\n" +
" └─ Table\n" +
" ├─ name: mytable\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM (SELECT i FROM mytable LIMIT 1) sq1) sq2 WHERE i = 3;`,
ExpectedPlan: "SubqueryAlias\n" +
" ├─ name: sq2\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq1\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Filter\n" +
" ├─ Eq\n" +
" │ ├─ mytable.i:0!null\n" +
" │ └─ 3 (tinyint)\n" +
" └─ Limit(1)\n" +
" └─ Table\n" +
" ├─ name: mytable\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC LIMIT 1) sq WHERE i = 3;`,
ExpectedPlan: "SubqueryAlias\n" +
" ├─ name: sq\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Filter\n" +
" ├─ Eq\n" +
" │ ├─ mytable.i:0!null\n" +
" │ └─ 3 (tinyint)\n" +
" └─ Limit(1)\n" +
" └─ TopN(Limit: [1 (tinyint)]; mytable.i:0!null DESC nullsFirst)\n" +
" └─ Table\n" +
" ├─ name: mytable\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM (SELECT i FROM mytable ORDER BY i DESC LIMIT 1) sq1) sq2 WHERE i = 3;`,
ExpectedPlan: "SubqueryAlias\n" +
" ├─ name: sq2\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq1\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Filter\n" +
" ├─ Eq\n" +
" │ ├─ mytable.i:0!null\n" +
" │ └─ 3 (tinyint)\n" +
" └─ Limit(1)\n" +
" └─ TopN(Limit: [1 (tinyint)]; mytable.i:0!null DESC nullsFirst)\n" +
" └─ Table\n" +
" ├─ name: mytable\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM mytable WHERE i > 1) sq LIMIT 1;`,
ExpectedPlan: "Limit(1)\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ IndexedTableAccess(mytable)\n" +
" ├─ index: [mytable.i]\n" +
" ├─ static: [{(1, ∞)}]\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM (SELECT i FROM mytable WHERE i > 1) sq1) sq2 LIMIT 1;`,
ExpectedPlan: "Limit(1)\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq2\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq1\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ IndexedTableAccess(mytable)\n" +
" ├─ index: [mytable.i]\n" +
" ├─ static: [{(1, ∞)}]\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM (SELECT i FROM mytable) sq1 WHERE i > 1) sq2 LIMIT 1;`,
ExpectedPlan: "Limit(1)\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq2\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq1\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ IndexedTableAccess(mytable)\n" +
" ├─ index: [mytable.i]\n" +
" ├─ static: [{(1, ∞)}]\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT i FROM (SELECT i FROM (SELECT i FROM mytable LIMIT 1) sq1 WHERE i > 1) sq2 LIMIT 10;`,
ExpectedPlan: "Limit(10)\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq2\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ SubqueryAlias\n" +
" ├─ name: sq1\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Filter\n" +
" ├─ GreaterThan\n" +
" │ ├─ mytable.i:0!null\n" +
" │ └─ 1 (tinyint)\n" +
" └─ Limit(1)\n" +
" └─ Table\n" +
" ├─ name: mytable\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT * FROM (SELECT a.pk, b.i FROM one_pk a JOIN mytable b ORDER BY a.pk ASC, b.i ASC LIMIT 1) sq WHERE i != 0`,
ExpectedPlan: "SubqueryAlias\n" +
" ├─ name: sq\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Filter\n" +
" ├─ NOT\n" +
" │ └─ Eq\n" +
" │ ├─ b.i:1!null\n" +
" │ └─ 0 (tinyint)\n" +
" └─ Limit(1)\n" +
" └─ TopN(Limit: [1 (tinyint)]; a.pk:0!null ASC nullsFirst, b.i:1!null ASC nullsFirst)\n" +
" └─ CrossJoin\n" +
" ├─ TableAlias(a)\n" +
" │ └─ Table\n" +
" │ ├─ name: one_pk\n" +
" │ └─ columns: [pk]\n" +
" └─ TableAlias(b)\n" +
" └─ Table\n" +
" ├─ name: mytable\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT * FROM (SELECT a.pk, b.i FROM one_pk a JOIN mytable b ORDER BY a.pk DESC, b.i DESC LIMIT 1) sq WHERE i != 0`,
ExpectedPlan: "SubqueryAlias\n" +
" ├─ name: sq\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Filter\n" +
" ├─ NOT\n" +
" │ └─ Eq\n" +
" │ ├─ b.i:1!null\n" +
" │ └─ 0 (tinyint)\n" +
" └─ Limit(1)\n" +
" └─ TopN(Limit: [1 (tinyint)]; a.pk:0!null DESC nullsFirst, b.i:1!null DESC nullsFirst)\n" +
" └─ CrossJoin\n" +
" ├─ TableAlias(a)\n" +
" │ └─ Table\n" +
" │ ├─ name: one_pk\n" +
" │ └─ columns: [pk]\n" +
" └─ TableAlias(b)\n" +
" └─ Table\n" +
" ├─ name: mytable\n" +
" └─ columns: [i]\n" +
"",
},
{
Query: `SELECT * FROM (SELECT pk FROM one_pk WHERE pk < 2 LIMIT 1) a JOIN (SELECT i FROM mytable WHERE i > 1 LIMIT 1) b WHERE pk >= 2;`,
ExpectedPlan: "CrossJoin\n" +
" ├─ SubqueryAlias\n" +
" │ ├─ name: a\n" +
" │ ├─ outerVisibility: false\n" +
" │ ├─ cacheable: true\n" +
" │ └─ Filter\n" +
" │ ├─ GreaterThanOrEqual\n" +
" │ │ ├─ one_pk.pk:0!null\n" +
" │ │ └─ 2 (tinyint)\n" +
" │ └─ Limit(1)\n" +
" │ └─ Filter\n" +
" │ ├─ LessThan\n" +
" │ │ ├─ one_pk.pk:0!null\n" +
" │ │ └─ 2 (tinyint)\n" +
" │ └─ Table\n" +
" │ ├─ name: one_pk\n" +
" │ └─ columns: [pk]\n" +
" └─ SubqueryAlias\n" +
" ├─ name: b\n" +
" ├─ outerVisibility: false\n" +
" ├─ cacheable: true\n" +
" └─ Limit(1)\n" +
" └─ IndexedTableAccess(mytable)\n" +
" ├─ index: [mytable.i]\n" +
" ├─ static: [{(1, ∞)}]\n" +
" └─ columns: [i]\n" +
"",
},
}

// QueryPlanTODOs are queries where the query planner produces a correct (results) but suboptimal plan.
Expand Down
10 changes: 10 additions & 0 deletions sql/analyzer/pushdown.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ func canDoPushdown(n sql.Node) bool {
// the secondary table below the join, we end up not evaluating it in all cases (since the secondary table result is
// sometimes null in these types of joins). It must be evaluated only after the join result is computed.
func filterPushdownChildSelector(c transform.Context) bool {
switch c.Node.(type) {
case *plan.Limit:
return false
}

switch n := c.Parent.(type) {
case *plan.TableAlias:
return false
Expand Down Expand Up @@ -413,6 +418,11 @@ func convertFiltersToIndexedAccess(
// pushdown, it will get picked up in the isolated pass
// run by the filters pushdown transform.
return false
case *plan.Filter:
// Can't push Filter Nodes below Limit Nodes
if _, ok := c.Node.(*plan.Limit); ok {
return false
}
}
return true
}
Expand Down