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

Jump to bottom

Spark3 join types #1942

Merged
merged 6 commits into from Nov 19, 2021
Merged

Spark3 join types #1942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bc61ad1
Add Spark3 join types
jpy-git Nov 19, 2021
2104ad7
black format and linting
jpy-git Nov 19, 2021
4ba8152
Merge branch 'main' into spark3_join_types
jpy-git Nov 19, 2021
6f8fde8
Merge branch 'main' into spark3_join_types
jpy-git Nov 19, 2021
668c34a
replace get_eventual_alias with method from ansi
jpy-git Nov 19, 2021
16022f5
Merge branch 'main' into spark3_join_types
jpy-git Nov 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
80 changes: 79 additions & 1 deletion src/sqlfluff/dialects/dialect_spark3.py
View file
Open in desktop
Expand Up @@ -15,8 +15,11 @@
AnyNumberOf,
BaseSegment,
Bracketed,
Delimited,
CommentSegment,
Conditional,
Dedent,
Delimited,
Indent,
NamedParser,
OneOf,
OptionallyBracketed,
Expand Down Expand Up @@ -693,3 +696,78 @@ class StatementSegment(BaseSegment):
Ref("DropModelStatementSegment"),
],
)


@spark3_dialect.segment(replace=True)
class JoinClauseSegment(BaseSegment):
Copy link
Member

Choose a reason for hiding this comment

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

Can't you just do this:

spark3_dialect.replace(
    JoinKeywords=Sequence(OneOf("SEMI", "ANTI", optional=True), "JOIN"),
)
`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tunetheweb looking in ansi JoinKeywords is just the word JOIN,the JOIN types (LEFT, RIGHT, INNER etc) are the same as I've done here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think adding those to JOIN keywords would allow things like CROSS SEMI JOIN which isn't possible

Copy link
Member

Choose a reason for hiding this comment

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

Yes but you don’t need to override the JoinClauseSegment at all - just the JoinKeywords So your 80 line change becomes just the above 3 lines.

Copy link
Member

Choose a reason for hiding this comment

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

Think adding those to JOIN keywords would allow things like CROSS SEMI JOIN which isn't possible

That is true. Your change is more technically accurate since it won’t allow that.

On the other hand, none of our dialects are 100% accurate to the SQL syntax and how likely is it above is used?

To me the point of implementing the parse is so we can implement rules that depend on understanding a SQL component rather than necessarily being 100% accurate to to the syntax. We should look to enforce coding standards and pick up silent syntax errors where ever possible. That SQL would not run so will be picked up. Yes it would be nice if linter picked up these things but I don’t think it’s essential.

The question is, whether the simple 3 line fix over the 80 line one, is simpler, sufficient and the downsides are acceptable? Or do we go the whole copying route and duplicate the code?

I could go either way to be honest. Obviously we override lots of things so another one isn’t an issue. Just saying I probably would have gone the simpler route myself if I’d done this. But maybe I’m just lazy 😀

Copy link
Contributor Author

@jpy-git jpy-git Nov 19, 2021 •
edited

Choose a reason for hiding this comment

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

I guess for me given it's more accurate to the true syntax and also consistent with the placement of the other join types the 80 lines is fine with me.
Remember you may want to add other non-ansi segments (e.g. natural joins) to this segment in the future so I think it pays to be explicit with the syntax when we can.
I tend not to worry too much about low line count for the sake of it.
"Explicit is better than implicit" 😄

"""Any number of join clauses, including the `JOIN` keyword.

https://spark.apache.org/docs/3.0.0/sql-ref-syntax-qry-select-join.html
TODO: Add NATURAL JOIN syntax.
"""

type = "join_clause"
match_grammar = Sequence(
# NB These qualifiers are optional
# TODO: Allow nested joins like:
# ....FROM S1.T1 t1 LEFT JOIN ( S2.T2 t2 JOIN S3.T3 t3 ON t2.col1=t3.col1) ON tab1.col1 = tab2.col1
OneOf(
"CROSS",
"INNER",
Sequence(
OneOf(
"FULL",
"LEFT",
"RIGHT",
),
Ref.keyword("OUTER", optional=True),
),
Sequence(
Ref.keyword("LEFT", optional=True),
"SEMI",
),
Sequence(
Ref.keyword("LEFT", optional=True),
"ANTI",
),
optional=True,
),
Ref("JoinKeywords"),
Indent,
Sequence(
Ref("FromExpressionElementSegment"),
Conditional(Dedent, indented_using_on=False),
# NB: this is optional
OneOf(
# ON clause
Ref("JoinOnConditionSegment"),
# USING clause
Sequence(
"USING",
Indent,
Bracketed(
# NB: We don't use BracketedColumnReferenceListGrammar
# here because we're just using SingleIdentifierGrammar,
# rather than ObjectReferenceSegment or ColumnReferenceSegment.
# This is a) so that we don't lint it as a reference and
# b) because the column will probably be returned anyway
# during parsing.
Delimited(
Ref("SingleIdentifierGrammar"),
ephemeral_name="UsingClauseContents",
)
),
Dedent,
),
# Unqualified joins *are* allowed. They just might not
# be a good idea.
optional=True,
),
Conditional(Indent, indented_using_on=False),
),
Dedent,
)

get_eventual_alias = ansi_dialect.get_segment(
"JoinClauseSegment"
).get_eventual_alias
4 changes: 4 additions & 0 deletions test/fixtures/dialects/spark3/join_types.sql
View file
Open in desktop
@@ -0,0 +1,4 @@
SELECT * FROM employee SEMI JOIN department ON employee.deptno = department.deptno;
SELECT * FROM employee ANTI JOIN department ON employee.deptno = department.deptno;
SELECT * FROM employee LEFT SEMI JOIN department ON employee.deptno = department.deptno;
SELECT * FROM employee LEFT ANTI JOIN department ON employee.deptno = department.deptno;
151 changes: 151 additions & 0 deletions test/fixtures/dialects/spark3/join_types.yml
View file
Open in desktop
@@ -0,0 +1,151 @@
# YML test files are auto-generated from SQL files and should not be edited by
# hand. To help enforce this, the "hash" field in the file must match a hash
# computed by SQLFluff when running the tests. Please run
# `python test/generate_parse_fixture_yml.py` to generate them after adding or
# altering SQL files.
_hash: 083b7cabc2d0da7873019cf780c8a8e0046fd187223d5da951159dbaf4d03ca4
file:
- base:
select_statement:
select_clause:
keyword: SELECT
select_clause_element:
wildcard_expression:
wildcard_identifier:
star: '*'
from_clause:
keyword: FROM
from_expression:
from_expression_element:
table_expression:
table_reference:
identifier: employee
alias_expression:
identifier: SEMI
join_clause:
keyword: JOIN
from_expression_element:
table_expression:
table_reference:
identifier: department
join_on_condition:
keyword: 'ON'
expression:
- column_reference:
- identifier: employee
- dot: .
- identifier: deptno
- comparison_operator: '='
- column_reference:
- identifier: department
- dot: .
- identifier: deptno
- statement_terminator: ;
- base:
select_statement:
select_clause:
keyword: SELECT
select_clause_element:
wildcard_expression:
wildcard_identifier:
star: '*'
from_clause:
keyword: FROM
from_expression:
from_expression_element:
table_expression:
table_reference:
identifier: employee
alias_expression:
identifier: ANTI
join_clause:
keyword: JOIN
from_expression_element:
table_expression:
table_reference:
identifier: department
join_on_condition:
keyword: 'ON'
expression:
- column_reference:
- identifier: employee
- dot: .
- identifier: deptno
- comparison_operator: '='
- column_reference:
- identifier: department
- dot: .
- identifier: deptno
- statement_terminator: ;
- base:
select_statement:
select_clause:
keyword: SELECT
select_clause_element:
wildcard_expression:
wildcard_identifier:
star: '*'
from_clause:
keyword: FROM
from_expression:
from_expression_element:
table_expression:
table_reference:
identifier: employee
join_clause:
- keyword: LEFT
- keyword: SEMI
- keyword: JOIN
- from_expression_element:
table_expression:
table_reference:
identifier: department
- join_on_condition:
keyword: 'ON'
expression:
- column_reference:
- identifier: employee
- dot: .
- identifier: deptno
- comparison_operator: '='
- column_reference:
- identifier: department
- dot: .
- identifier: deptno
- statement_terminator: ;
- base:
select_statement:
select_clause:
keyword: SELECT
select_clause_element:
wildcard_expression:
wildcard_identifier:
star: '*'
from_clause:
keyword: FROM
from_expression:
from_expression_element:
table_expression:
table_reference:
identifier: employee
join_clause:
- keyword: LEFT
- keyword: ANTI
- keyword: JOIN
- from_expression_element:
table_expression:
table_reference:
identifier: department
- join_on_condition:
keyword: 'ON'
expression:
- column_reference:
- identifier: employee
- dot: .
- identifier: deptno
- comparison_operator: '='
- column_reference:
- identifier: department
- dot: .
- identifier: deptno
- statement_terminator: ;