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

hashjoin indexing #1728

Merged
merged 31 commits into from
Apr 27, 2023
Merged

hashjoin indexing #1728

Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
eee289b
some tests
max-hoffman Apr 20, 2023
ee224a5
add changes
max-hoffman Apr 20, 2023
521bb1a
very close
Apr 21, 2023
f93f3d4
remove debugging code
Apr 21, 2023
dfbd936
another
Apr 21, 2023
5b8915c
aaaaaaaaaaaa
Apr 21, 2023
87b5a9d
it's there because I told it to print it
Apr 21, 2023
88b1d10
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 21, 2023
e2ef855
adding plan test
Apr 24, 2023
da0ceb4
tests
Apr 24, 2023
61c8b43
Merge branch 'james/cte3' of https://github.com/dolthub/go-mysql-serv…
Apr 24, 2023
410b82e
skip test
Apr 24, 2023
b2e52c8
aaaaaaaaaaaaa
Apr 24, 2023
59d1484
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 24, 2023
98aa934
moved to keyless test
Apr 24, 2023
d261dd8
make script test
Apr 24, 2023
118dcae
remove old code
Apr 24, 2023
783fc06
asdf
Apr 24, 2023
c778cf3
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 24, 2023
db7fcd0
asdf
Apr 24, 2023
0d34d51
skip prepared
Apr 24, 2023
8a4eebe
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 24, 2023
1e039b4
adding broken query
Apr 24, 2023
6a981ed
fixing expected result
Apr 24, 2023
3917790
[ga-format-pr] Run ./format_repo.sh to fix formatting
jcor11599 Apr 24, 2023
9821fe3
work this time
Apr 25, 2023
b801fd7
simplified
Apr 25, 2023
51c5d0a
copy scope
Apr 26, 2023
b8b7e57
no need to check for isleftouter
Apr 26, 2023
117063b
comments and more tests
Apr 26, 2023
b85582c
copy method
Apr 26, 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
88 changes: 88 additions & 0 deletions enginetest/queries/queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7522,6 +7522,71 @@ SELECT * FROM my_cte;`,
{2},
},
},
{
// Original Issue: https://github.com/dolthub/dolt/issues/5714
Query: `
Copy link
Contributor

Choose a reason for hiding this comment

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

a comment tagging the original issue might be worthwhile


SELECT COUNT(*)
FROM keyless
WHERE keyless.c0 IN (

WITH RECURSIVE cte(depth, i, j) AS (
SELECT 0, T1.c0, T1.c1
FROM keyless T1
WHERE T1.c0 = 0

UNION ALL

SELECT cte.depth + 1, cte.i, T2.c1 + 1
FROM cte, keyless T2
WHERE cte.depth = T2.c0
)

SELECT U0.c0
FROM keyless U0, cte
WHERE cte.j = keyless.c0

)
ORDER BY c0;
;`,

Expected: []sql.Row{
{4},
},
},
{
// Original Issue: https://github.com/dolthub/dolt/issues/5714
// Similar, but this time the subquery is on the left
Query: `

SELECT COUNT(*)
FROM keyless
WHERE keyless.c0 IN (

WITH RECURSIVE cte(depth, i, j) AS (
SELECT 0, T1.c0, T1.c1
FROM keyless T1
WHERE T1.c0 = 0

UNION ALL

SELECT cte.depth + 1, cte.i, T2.c1 + 1
FROM cte, keyless T2
WHERE cte.depth = T2.c0
)

SELECT U0.c0
FROM cte, keyless U0
WHERE cte.j = keyless.c0

)
ORDER BY c0;
;`,

Expected: []sql.Row{
{4},
},
},
}

var KeylessQueries = []QueryTest{
Expand Down Expand Up @@ -7946,6 +8011,29 @@ var BrokenQueries = []QueryTest{
Query: "select 2000.0 / 250000000.0 * (24.0 * 6.0 * 6.25 * 10.0);",
Expected: []sql.Row{{"0.0720000000"}},
},
{
// This panics
// The non-recursive part of the UNION ALL returns too many rows, causing index out of bounds errors
// Without the join on mytable and cte, this error is caught
Query: `
WITH RECURSIVE cte(i, j) AS (
SELECT 0, 1, 2
FROM mytable

UNION ALL

SELECT *
FROM mytable, cte
WHERE cte.i = mytable.i
)
SELECT *
FROM mytable;`,
Expected: []sql.Row{
{1, "first row"},
{2, "second row"},
{3, "third row"},
},
},
}

var VersionedQueries = []QueryTest{
Expand Down
156 changes: 156 additions & 0 deletions enginetest/queries/query_plans.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7694,6 +7694,162 @@ With c as (
" └─ columns: [i]\n" +
"",
},
{
Query: `
SELECT COUNT(*)
FROM keyless
WHERE keyless.c0 IN (
WITH RECURSIVE cte(depth, i, j) AS (
SELECT 0, T1.c0, T1.c1
FROM keyless T1
WHERE T1.c0 = 0

UNION ALL

SELECT cte.depth + 1, cte.i, T2.c1 + 1
FROM cte, keyless T2
WHERE cte.depth = T2.c0
)

SELECT U0.c0
FROM keyless U0, cte
WHERE cte.j = keyless.c0
);`,
ExpectedPlan: "Project\n" +
" ├─ columns: [COUNT(1):0!null as COUNT(*)]\n" +
" └─ GroupBy\n" +
" ├─ select: COUNT(1 (bigint))\n" +
" ├─ group: \n" +
" └─ Filter\n" +
" ├─ InSubquery\n" +
" │ ├─ left: keyless.c0:0\n" +
" │ └─ right: Subquery\n" +
" │ ├─ cacheable: false\n" +
" │ └─ Project\n" +
" │ ├─ columns: [U0.c0:2]\n" +
" │ └─ Filter\n" +
" │ ├─ Eq\n" +
" │ │ ├─ cte.j:5\n" +
" │ │ └─ keyless.c0:0\n" +
" │ └─ CrossJoin\n" +
" │ ├─ TableAlias(U0)\n" +
" │ │ └─ Table\n" +
" │ │ ├─ name: keyless\n" +
" │ │ └─ columns: [c0]\n" +
" │ └─ SubqueryAlias\n" +
" │ ├─ name: cte\n" +
" │ ├─ outerVisibility: true\n" +
" │ ├─ cacheable: true\n" +
" │ └─ RecursiveCTE\n" +
" │ └─ Union all\n" +
" │ ├─ Project\n" +
" │ │ ├─ columns: [0 (tinyint), T1.c0:2, T1.c1:3]\n" +
" │ │ └─ Filter\n" +
" │ │ ├─ Eq\n" +
" │ │ │ ├─ T1.c0:2\n" +
" │ │ │ └─ 0 (tinyint)\n" +
" │ │ └─ TableAlias(T1)\n" +
" │ │ └─ Table\n" +
" │ │ ├─ name: keyless\n" +
" │ │ └─ columns: [c0 c1]\n" +
" │ └─ Project\n" +
" │ ├─ columns: [(cte.depth:3!null + 1 (tinyint)), cte.i:4, (T2.c1:7 + 1 (tinyint))]\n" +
" │ └─ HashJoin\n" +
" │ ├─ Eq\n" +
" │ │ ├─ cte.depth:3!null\n" +
" │ │ └─ T2.c0:6\n" +
" │ ├─ RecursiveTable(cte)\n" +
" │ └─ HashLookup\n" +
" │ ├─ source: TUPLE(cte.depth:3!null)\n" +
" │ ├─ target: TUPLE(T2.c0:2)\n" +
" │ └─ CachedResults\n" +
" │ └─ TableAlias(T2)\n" +
" │ └─ Table\n" +
" │ ├─ name: keyless\n" +
" │ └─ columns: [c0 c1]\n" +
" └─ Table\n" +
" ├─ name: keyless\n" +
" └─ columns: [c0 c1]\n" +
"",
},
{
Query: `
SELECT COUNT(*)
FROM keyless
WHERE keyless.c0 IN (
WITH RECURSIVE cte(depth, i, j) AS (
SELECT 0, T1.c0, T1.c1
FROM keyless T1
WHERE T1.c0 = 0

UNION ALL

SELECT cte.depth + 1, cte.i, T2.c1 + 1
FROM cte, keyless T2
WHERE cte.depth = T2.c0
)

SELECT U0.c0
FROM cte, keyless U0
WHERE cte.j = keyless.c0
);`,
ExpectedPlan: "Project\n" +
" ├─ columns: [COUNT(1):0!null as COUNT(*)]\n" +
" └─ GroupBy\n" +
" ├─ select: COUNT(1 (bigint))\n" +
" ├─ group: \n" +
" └─ Filter\n" +
" ├─ InSubquery\n" +
" │ ├─ left: keyless.c0:0\n" +
" │ └─ right: Subquery\n" +
" │ ├─ cacheable: false\n" +
" │ └─ Project\n" +
" │ ├─ columns: [U0.c0:5]\n" +
" │ └─ Filter\n" +
" │ ├─ Eq\n" +
" │ │ ├─ cte.j:4\n" +
" │ │ └─ keyless.c0:0\n" +
" │ └─ CrossJoin\n" +
" │ ├─ SubqueryAlias\n" +
" │ │ ├─ name: cte\n" +
" │ │ ├─ outerVisibility: true\n" +
" │ │ ├─ cacheable: true\n" +
" │ │ └─ RecursiveCTE\n" +
" │ │ └─ Union all\n" +
" │ │ ├─ Project\n" +
" │ │ │ ├─ columns: [0 (tinyint), T1.c0:2, T1.c1:3]\n" +
" │ │ │ └─ Filter\n" +
" │ │ │ ├─ Eq\n" +
" │ │ │ │ ├─ T1.c0:2\n" +
" │ │ │ │ └─ 0 (tinyint)\n" +
" │ │ │ └─ TableAlias(T1)\n" +
" │ │ │ └─ Table\n" +
" │ │ │ ├─ name: keyless\n" +
" │ │ │ └─ columns: [c0 c1]\n" +
" │ │ └─ Project\n" +
" │ │ ├─ columns: [(cte.depth:2!null + 1 (tinyint)), cte.i:3, (T2.c1:6 + 1 (tinyint))]\n" +
" │ │ └─ HashJoin\n" +
" │ │ ├─ Eq\n" +
" │ │ │ ├─ cte.depth:2!null\n" +
" │ │ │ └─ T2.c0:5\n" +
" │ │ ├─ RecursiveTable(cte)\n" +
" │ │ └─ HashLookup\n" +
" │ │ ├─ source: TUPLE(cte.depth:2!null)\n" +
" │ │ ├─ target: TUPLE(T2.c0:2)\n" +
" │ │ └─ CachedResults\n" +
" │ │ └─ TableAlias(T2)\n" +
" │ │ └─ Table\n" +
" │ │ ├─ name: keyless\n" +
" │ │ └─ columns: [c0 c1]\n" +
" │ └─ TableAlias(U0)\n" +
" │ └─ Table\n" +
" │ ├─ name: keyless\n" +
" │ └─ columns: [c0]\n" +
" └─ Table\n" +
" ├─ name: keyless\n" +
" └─ columns: [c0 c1]\n" +
"",
},
}

// QueryPlanTODOs are queries where the query planner produces a correct (results) but suboptimal plan.
Expand Down
1 change: 0 additions & 1 deletion sql/analyzer/analyzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,6 @@ func postPrepareRuleSelector(id RuleId) bool {
stripTableNameInDefaultsId,
resolvePreparedInsertId,
finalizeSubqueriesId,
finalizeUnionsId,

// DefaultValidationRules
validateResolvedId,
Expand Down
7 changes: 6 additions & 1 deletion sql/analyzer/exec_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,12 @@ func (b *ExecBuilder) buildHashJoin(j *hashJoin, input sql.Schema, children ...s
if err != nil {
return nil, err
}
outerAttrs, err := b.buildFilters(j.g.m.scope, j.right.relProps.OutputCols(), expression.Tuple(j.outerAttrs))
tmpScope := j.g.m.scope
if tmpScope != nil {
tmpScope = tmpScope.newScopeInJoin(nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

could we have slightly more expressive methods? like j.g.m.scope.newScopeNoJoin()?

tmpScope.joinSiblings = []sql.Node{}
}
outerAttrs, err := b.buildFilters(tmpScope, j.right.relProps.OutputCols(), expression.Tuple(j.outerAttrs))
if err != nil {
return nil, err
}
Expand Down
1 change: 1 addition & 0 deletions sql/analyzer/indexed_joins.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ func inOrderReplanJoin(

// two different base cases, depending on whether we reorder or not
if reorder {
scope.SetJoin(true)
ret, err := replanJoin(ctx, j, a, scope)
if err != nil {
return nil, transform.SameTree, fmt.Errorf("failed to replan join: %w", err)
Expand Down
3 changes: 2 additions & 1 deletion sql/analyzer/pushdown.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,8 @@ func convertIsNullForIndexes(ctx *sql.Context, e sql.Expression) sql.Expression
// pushdownFixIndices fixes field indices for non-join expressions (replanJoin
// is responsible for join filters and conditions.)
func pushdownFixIndices(a *Analyzer, n sql.Node, scope *Scope) (sql.Node, transform.TreeIdentity, error) {
if _, ok := n.(*plan.JoinNode); ok {
switch n := n.(type) {
case *plan.JoinNode, *plan.HashLookup:
return n, transform.SameTree, nil
}
return FixFieldIndexesForExpressions(a, n, scope)
Expand Down
Loading