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

opt: add support for LATERAL in FROM clause #36613

Merged
merged 1 commit into from Apr 16, 2019

Conversation

Projects
None yet
3 participants
@justinj
Copy link
Member

commented Apr 8, 2019

Fixes #24676.
Partial fix for #24560.

This commit adds support for LATERAL in the FROM clause of a SELECT.
LATERAL allows a subquery or SRF to refer to columns in tables earlier
in the FROM clause. This is semantically an inner apply join.

A hiccup with this is that we build join trees in a FROM clause
right-deep, but the semantics of LATERAL force the tree to be left-deep.
Changing the default to be left-deep could cause some hand-optimized
queries to become slower, so we only change the order if there is a
LATERAL subquery. Note that SRFs are always implicitly LATERAL.

This commit does not add support for LATERAL as a keyword to explicit
JOIN expressions.

Release note (sql change): the LATERAL keyword in a FROM clause is now
supported.

@justinj justinj requested review from rytaft and RaduBerinde Apr 8, 2019

@justinj justinj requested review from cockroachdb/sql-execution-prs as code owners Apr 8, 2019

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

This change is Reviewable

@rytaft

rytaft approved these changes Apr 8, 2019

Copy link
Contributor

left a comment

:lgtm:

Reviewed 14 of 14 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @justinj and @RaduBerinde)


pkg/sql/opt/optbuilder/select.go, line 774 at r1 (raw file):

		return outScope
	}
	tableScope := b.buildFromTables(tables, inScope)

You already checked that there are no lateral expressions, right? So I think you can call buildFromTablesRightDeep here.


pkg/sql/opt/optbuilder/select.go, line 794 at r1 (raw file):

		return false
	}
	// Expressions which are explicitly lateral are lateral.

how about "Expressions which explicitly use the LATERAL keyword are lateral"


pkg/sql/opt/optbuilder/select.go, line 804 at r1 (raw file):

// buildFromWithLateral builds a FROM clause in the case where it contains a
// LATERAL table.  This differs from buildFromTables because the semantics of

[nit] buildFromTables -> buildFromTablesRightDeep


pkg/sql/opt/optbuilder/testdata/inner-join, line 140 at r1 (raw file):

error (42712): source name "a" specified more than once (missing AS clause)

# TODO(justin): this case should be rejected for having a name specified twice.

shouldn't this have been caught by validateJoinTableNames?


pkg/sql/opt/optbuilder/testdata/lateral, line 173 at r1 (raw file):


build
SELECT * FROM x, LATERAL (SELECT * FROM y WHERE b = a), z

this looks like a duplicate


pkg/sql/opt/optbuilder/testdata/lateral, line 282 at r1 (raw file):

 └── filters (true)

opt

did you mean to use opt here?

@justinj justinj force-pushed the justinj:lateral branch from 3ae3f56 to eccbb45 Apr 15, 2019

opt: add support for LATERAL in FROM clause
Fixes #24676.
Partial fix for #24560.

This commit adds support for LATERAL in the FROM clause of a SELECT.
LATERAL allows a subquery or SRF to refer to columns in tables earlier
in the FROM clause. This is semantically an inner apply join.

A hiccup with this is that we build join trees in a FROM clause
right-deep, but the semantics of LATERAL force the tree to be left-deep.
Changing the default to be left-deep could cause some hand-optimized
queries to become slower, so we only change the order if there is a
LATERAL subquery. Note that SRFs are always implicitly LATERAL.

This commit does *not* add support for LATERAL as a keyword to explicit
`JOIN` expressions.

Release note (sql change): the LATERAL keyword in a FROM clause is now
supported.

@justinj justinj force-pushed the justinj:lateral branch from eccbb45 to 4bd4882 Apr 16, 2019

@justinj
Copy link
Member Author

left a comment

TFTR! Sorry for the delay in getting this wrapped up.

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @RaduBerinde and @rytaft)


pkg/sql/opt/optbuilder/select.go, line 774 at r1 (raw file):

Previously, rytaft wrote…

You already checked that there are no lateral expressions, right? So I think you can call buildFromTablesRightDeep here.

Good call, done.


pkg/sql/opt/optbuilder/select.go, line 794 at r1 (raw file):

Previously, rytaft wrote…

how about "Expressions which explicitly use the LATERAL keyword are lateral"

Done.


pkg/sql/opt/optbuilder/select.go, line 804 at r1 (raw file):

Previously, rytaft wrote…

[nit] buildFromTables -> buildFromTablesRightDeep

Done.


pkg/sql/opt/optbuilder/testdata/inner-join, line 140 at r1 (raw file):

Previously, rytaft wrote…

shouldn't this have been caught by validateJoinTableNames?

It doesn't catch sources which aren't made up of literal base tables, it seems. This happened before this change, fwiw.


pkg/sql/opt/optbuilder/testdata/lateral, line 173 at r1 (raw file):

Previously, rytaft wrote…

this looks like a duplicate

Good catch!


pkg/sql/opt/optbuilder/testdata/lateral, line 282 at r1 (raw file):

Previously, rytaft wrote…

did you mean to use opt here?

Ah nope! I think I was curious if this would get decorrelated.

craig bot pushed a commit that referenced this pull request Apr 16, 2019

Merge #36613
36613: opt: add support for LATERAL in FROM clause r=justinj a=justinj

Fixes #24676.
Partial fix for #24560.

This commit adds support for LATERAL in the FROM clause of a SELECT.
LATERAL allows a subquery or SRF to refer to columns in tables earlier
in the FROM clause. This is semantically an inner apply join.

A hiccup with this is that we build join trees in a FROM clause
right-deep, but the semantics of LATERAL force the tree to be left-deep.
Changing the default to be left-deep could cause some hand-optimized
queries to become slower, so we only change the order if there is a
LATERAL subquery. Note that SRFs are always implicitly LATERAL.

This commit does *not* add support for LATERAL as a keyword to explicit
`JOIN` expressions.

Release note (sql change): the LATERAL keyword in a FROM clause is now
supported.

Co-authored-by: Justin Jaffray <justin@cockroachlabs.com>
@rytaft

rytaft approved these changes Apr 16, 2019

Copy link
Contributor

left a comment

Reviewed 5 of 5 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @RaduBerinde and @rytaft)

@craig

This comment has been minimized.

Copy link

commented Apr 16, 2019

Build failed

@justinj

This comment has been minimized.

Copy link
Member Author

commented Apr 16, 2019

looks like a vet oom, gonna try again

bors r+

craig bot pushed a commit that referenced this pull request Apr 16, 2019

Merge #36613
36613: opt: add support for LATERAL in FROM clause r=justinj a=justinj

Fixes #24676.
Partial fix for #24560.

This commit adds support for LATERAL in the FROM clause of a SELECT.
LATERAL allows a subquery or SRF to refer to columns in tables earlier
in the FROM clause. This is semantically an inner apply join.

A hiccup with this is that we build join trees in a FROM clause
right-deep, but the semantics of LATERAL force the tree to be left-deep.
Changing the default to be left-deep could cause some hand-optimized
queries to become slower, so we only change the order if there is a
LATERAL subquery. Note that SRFs are always implicitly LATERAL.

This commit does *not* add support for LATERAL as a keyword to explicit
`JOIN` expressions.

Release note (sql change): the LATERAL keyword in a FROM clause is now
supported.

Co-authored-by: Justin Jaffray <justin@cockroachlabs.com>
@craig

This comment has been minimized.

Copy link

commented Apr 16, 2019

Build succeeded

@craig craig bot merged commit 4bd4882 into cockroachdb:master Apr 16, 2019

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.