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.

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

More join types #1280

Merged
merged 13 commits into from
Sep 27, 2022
Merged

More join types #1280

merged 13 commits into from
Sep 27, 2022

Conversation

max-hoffman
Copy link
Contributor

@max-hoffman max-hoffman commented Sep 22, 2022

Add FullOuterJoin, SemiJoin, and AntiJoin.

None of these new join nodes are safe for join ordering transformations yet. They are explicitly excluded from join planning.

The getField indexes for these three nodes' join conditions deserve more consideration. I excluded them from auto-fixup after manually correcting join condition get fields for the appropriate schemas.

FullOuterJoin uses a union distinct execution operator, which is correct but a lot slower than a merge join-esque operator.

SemiJoin and AntiJoin rearrange subquery expression scopes. I separate resolve and finalizeSubqueryExpressions to perform decorrelation before predicate pushdown (where we were panicking on FixUpExpressions) and join ordering (we want to decorrelate scopes before join planning).

Other:

  • query plan tests added for exist hoisting edge cases i did not catch on first pass
  • fixed bug with CTE stars

@max-hoffman max-hoffman marked this pull request as ready for review September 23, 2022 21:42
Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

Mostly looks good, just a few comments

For future PRs, it's easier to review these when split into smaller functional chunks (although this wasn't egregious)

})
}

// pluckCorrelatedExistsSubquery scans a filter for [note] WHERE EXISTS, and then attempts to
Copy link
Member

Choose a reason for hiding this comment

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

what is [note]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

autocorrect i think, meant [NOT] WHERE EXISTS

sql/analyzer/hoist_select_exists.go Outdated Show resolved Hide resolved
sql/analyzer/hoist_select_exists.go Show resolved Hide resolved
sql/analyzer/resolve_ctes.go Outdated Show resolved Hide resolved
sql/plan/join.go Outdated
type JoinType uint16

const (
UnknownJoinType JoinType = iota // UnknownJoinType
Copy link
Member

Choose a reason for hiding this comment

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

nit: generally speaking we name this kind of enum constant with the type first, so that all the possible values sort next to eachother

sql/plan/join.go Outdated Show resolved Hide resolved
BinaryNode
Cond sql.Expression
Filter sql.Expression
Copy link
Member

Choose a reason for hiding this comment

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

Prefer cond, is there a particular reason you changed this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

filters are more mobile in the new join ordering algo, seems easier to just call them all filters. At some point i'd like to split these into a list of filters rather than an expression.JoinAnd(filters...), probably memoized, index selection doesn't have to be as painful as it is right now.

panic("join implementation should provide schema")
}

func constructNewJoin(base *joinBase) JoinNode {
Copy link
Member

Choose a reason for hiding this comment

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

Smart, I like it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:)

sql/plan/join.go Outdated Show resolved Hide resolved
@max-hoffman max-hoffman merged commit 8b178aa into main Sep 27, 2022
@max-hoffman max-hoffman deleted the max/more-joins branch September 27, 2022 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants