Skip to content

[mssql/oracle] Support CROSS/OUTER APPLY #120

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

Merged
merged 2 commits into from
Jun 19, 2019

Conversation

nickolay
Copy link
Contributor

From the commit message:

T-SQL (and Oracle) support non-standard syntax, which is similar in
functionality to LATERAL joins in ANSI and PostgreSQL
https://blog.jooq.org/tag/lateral-derived-table/: it allows to use
the columns from the tables defined to the left of APPLY in the
"derived tables" (subqueries) to the right of APPLY. Unlike ANSI
LATERAL (but like Postgres' implementation), APPLY is also used with
table-valued function calls.

Despite them being similar, we represent "APPLY" joins with
JoinOperators of its own (CrossApply and OuterApply). Doing
otherwise seemed like it would cause unnecessary confusion, as those
interested in dialect-specific parsing would probably not expect APPLY
being parsed as LATERAL, and those wanting to forbid non-standard SQL
would not be helped by this either.

This also renames existing JoinOperator::Cross -> CrossJoin to avoid
confusion with CrossApply.

This also includes a fix for the bad merge in #118.

nickolay added 2 commits June 19, 2019 02:45
T-SQL (and Oracle) support non-standard syntax, which is similar in
functionality to LATERAL joins in ANSI and PostgreSQL
<https://blog.jooq.org/tag/lateral-derived-table/>: it allows to use
the columns from the tables defined to the left of `APPLY` in the
"derived tables" (subqueries) to the right of `APPLY`. Unlike ANSI
LATERAL (but like Postgres' implementation), APPLY is also used with
table-valued function calls.

Despite them being similar, we represent "APPLY" joins with
`JoinOperator`s of its own (`CrossApply` and `OuterApply`). Doing
otherwise seemed like it would cause unnecessary confusion, as those
interested in dialect-specific parsing would probably not expect APPLY
being parsed as LATERAL, and those wanting to forbid non-standard SQL
would not be helped by this either.

This also renames existing JoinOperator::Cross -> CrossJoin to avoid
confusion with CrossApply.
@nickolay nickolay requested a review from benesch June 18, 2019 23:52
@coveralls
Copy link

Pull Request Test Coverage Report for Build 361

  • 24 of 24 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 358: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 361

  • 24 of 24 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 358: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

Copy link
Contributor

@benesch benesch left a comment

Choose a reason for hiding this comment

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

LGTM!

@benesch
Copy link
Contributor

benesch commented Jun 19, 2019

(And thanks for explicitly clarifying in your commit message why you chose not to unify lateral and apply—that was super helpful!)

@nickolay nickolay merged commit 3c401d5 into apache:master Jun 19, 2019
@nickolay nickolay deleted the pr/outer-apply branch June 19, 2019 10:26
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.

3 participants