Spark3: parse some VALUES clauses #2245

mcannamela · 2022-01-06T16:17:46Z

Brief summary of the change made

Makes progress on #2152.

VALUES clauses fail to parse in the spark3 dialect I believe because TableExpressionSegment does not include ValuesClauseSegment. I address that only for the spark3 dialect by including it, and also redefining some segments to address subsequent parsing failures.

At present, I believe I am covering most simple cases of VALUES used in FROM clauses, but not any of the optional directives likeORDER BY, LIMIT, OFFSET that you can see in e.g. the postgres docs (Spark SQL docs don't mention VALUES except in INSERT statements). I suspect this should all apply to the ansi dialect but I must admit I gave up trying to find the spec after not that much looking.

Also, it only works reliably when the VALUES clause is in parentheses:

    select * from (values 1, 2) --works 
    select * from values 1, 2 --fails

I think this is because the parse_grammar of FromClauseSegment is Delimited(Ref("FromExpressionSegment")), which splits on the first comma it finds rather than looking for the longest FromExpressionSegment. I don't see a good way around that. I could live with this, but I wish I had a way to hint to the user that this is what went wrong, since the output of parse does not make this at all obvious that just wrapping with parens will fix it.

Are there any other side effects of this change that we should be aware of?

The only dialect tests that fail for spark3 are ones that I added, but since I do override TableExpressionSegment for spark3 I imagine there are tests we were relying on in ansi that we would want to make sure spark3 passes. Before going nuts with that, I wanted to see whether I shouldn't just be pushing all this down into the ansi dialect anyway.

Pull Request checklist

Please confirm you have completed any of the necessary steps below.
Included test cases to demonstrate any code changes, which may be one or more of the following:
- .yml rule test cases in test/fixtures/rules/std_rule_cases.
- .sql/.yml parser test cases in test/fixtures/dialects (note YML files can be auto generated with tox -e generate-fixture-yml).
- Full autofix test cases in test/fixtures/linter/autofix.
- Other.
Added appropriate documentation for the change.
Created GitHub issues for any relevant followup/future enhancements if appropriate.

…we are

src/sqlfluff/dialects/dialect_spark3.py

test/conftest.py

test/dialects/spark3_grammar_test.py

mcannamela · 2022-01-06T16:52:06Z

test/dialects/spark3_grammar_test.py

+        logger = logging.getLogger()
+        logger.addHandler(logging.StreamHandler())
+        for _, _, message in caplog.record_tuples:
+            logger.warning(message)


for my debugging purposes.

i wanted to ask, is there a good place that explains how to read the detailed parsing output?

just by staring at it i could see it was only picking up part of from values 1,2 as the FromExpressionSegment, but beyond that i was having trouble reading it:

[PD:0 MD:1 ] .None Ref.match OUT + [match=<MatchResult 1/9: 'from'>, seg="'from values 1 , 2'"] [PD:0 MD:2 ] ..None Delim._look_ahead_match IN [ls=7, seg='values 1 , 2'] [PD:0 MD:2 ] ..None Delim._look_ahead_match SI [mq=[], sb=['VALUES', '', '1', '', ',', '', '2']] [PD:0 MD:2 ] ..None Ref.match OUT + [match=<MatchResult 1/3: ','>, seg="', 2'"] [PD:0 MD:2 ] ..None Delim._look_ahead_match SC [bsm=(4, 1, <Ref: CommaSegment>)] [PD:0 MD:4 ] ....FromExpressionSegment OneOf.match PRN [ns=1, ps=1, ms=0, pruned=[<Ref: MLTableExpressionSegment>], opts=[<Ref: FromExpressionElementSegment>]] [PD:0 MD:7 ] .......FromExpressionElementSegment Ref.match OUT [match=<MatchResult 0/3: ''>, seg="'values 1'"] [PD:0 MD:7 ] .......FromExpressionElementSegment Optio.match PRN [ns=1, ps=1, ms=0, pruned=[<Bracketed: [<Ref: TableExpressionSegment>]>], opts=[<Ref: TableExpressionSegment>]] [PD:0 MD:9 ] .........TableExpressionSegment OneOf.match PRN [ns=3, ps=1, ms=1, pruned=[<Bracketed: [<Ref: SelectableGrammar>]>], opts=[<Ref: ValuesClauseSegment>, <Ref: BareFunctionSegment>, <Ref: FunctionSegment>, <Ref: TableReferenceSegment>]] [PD:0 MD:12] ............ValuesClauseSegment Ref.match OUT + [match=<MatchResult 1/3: 'values'>, seg="'values 1'"] [PD:0 MD:11] ...........ValuesClauseSegment Start.match OUT ++ [match=<MatchResult 3/3: 'values 1'>, seg="'values 1'"] [PD:0 MD:10] ..........ValuesClauseSegment Value.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:10] ..........TableExpressionSegment Ref.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:9 ] .........TableExpressionSegment OneOf.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:8 ] ........TableExpressionSegment Table.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:8 ] ........FromExpressionElementSegment Ref.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:7 ] .......FromExpressionElementSegment Optio.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:6 ] ......FromExpressionElementSegment Seque.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:5 ] .....FromExpressionElementSegment FromE.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:5 ] .....FromExpressionSegment Ref.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:4 ] ....FromExpressionSegment OneOf.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:3 ] ...FromExpressionSegment Condi.match OUT ++ [match=<MatchResult 1/1: ''>, seg="''"] [PD:0 MD:3 ] ...FromExpressionSegment Seque.match OUT ++ [match=<MatchResult 3/3: 'values 1'>, seg="'values 1'"] [PD:0 MD:2 ] ..FromExpressionSegment FromE.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:2 ] ..None Ref.match OUT ++ [match=<MatchResult 1/1: 'values 1'>, seg="'values 1'"] [PD:0 MD:2 ] ..None Delim._look_ahead_match IN [ls=2, seg=' 2'] [PD:0 MD:2 ] ..None Delim._look_ahead_match SI [mq=[], sb=['', '2']] [PD:0 MD:2 ] ..None Delim._look_ahead_match SC [bsm=None] [PD:0 MD:4 ] ....FromExpressionSegment OneOf.match PRN [ns=1, ps=1, ms=0, pruned=[<Ref: MLTableExpressionSegment>], opts=[<Ref: FromExpressionElementSegment>]] [PD:0 MD:7 ] .......FromExpressionElementSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:7 ] .......FromExpressionElementSegment Optio.match PRN [ns=1, ps=1, ms=0, pruned=[<Bracketed: [<Ref: TableExpressionSegment>]>], opts=[<Ref: TableExpressionSegment>]] [PD:0 MD:9 ] .........TableExpressionSegment OneOf.match PRN [ns=3, ps=2, ms=0, pruned=[<Ref: ValuesClauseSegment>, <Bracketed: [<Ref: SelectableGrammar>]>], opts=[<Ref: BareFunctionSegment>, <Ref: FunctionSegment>, <Ref: TableReferenceSegment>]] [PD:0 MD:10] ..........TableExpressionSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:11] ...........FunctionSegment OneOf.match PRN [ns=1, ps=1, ms=0, pruned=[<Sequence: [<Sequence: [<Ref: DatePartFunctionNameSe...]>], opts=[<Sequence: [<Sequence: [<AnyNumberOf: [<Ref: Functio..., <Ref: PostFunctionGrammar [opt]>]>]] [PD:0 MD:17] .................DatePartFunctionNameSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:16] ................DatePartFunctionNameSegment Seque.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:15] ...............DatePartFunctionNameSegment DateP.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:15] ...............FunctionSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:14] ..............FunctionSegment AnyNu.match PRN [ns=1, ps=0, ms=0, pruned=[], opts=[<Ref: FunctionNameSegment>]] [PD:0 MD:17] .................FunctionNameSegment AnyNu.match PRN [ns=1, ps=0, ms=0, pruned=[], opts=[<Sequence: [<Ref: SingleIdentifierGrammar>, <Ref: DotSegment>]>]] [PD:0 MD:19] ...................SingleIdentifierGrammar OneOf.match PRN [ns=2, ps=0, ms=0, pruned=[], opts=[<Ref: NakedIdentifierSegment>, <Ref: QuotedIdentifierSegment>]] [PD:0 MD:20] ....................SingleIdentifierGrammar Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:20] ....................SingleIdentifierGrammar Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:19] ...................SingleIdentifierGrammar OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:19] ...................FunctionNameSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:18] ..................FunctionNameSegment Seque.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:17] .................FunctionNameSegment AnyNu.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:17] .................FunctionNameSegment OneOf.match PRN [ns=2, ps=0, ms=0, pruned=[], opts=[<Ref: FunctionNameIdentifierSegment>, <Ref: QuotedIdentifierSegment>]] [PD:0 MD:18] ..................FunctionNameSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:18] ..................FunctionNameSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:17] .................FunctionNameSegment OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:16] ................FunctionNameSegment Seque.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:15] ...............FunctionNameSegment Funct.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:15] ...............FunctionSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:14] ..............FunctionSegment AnyNu.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:13] .............FunctionSegment Seque.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:12] ............FunctionSegment Seque.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:11] ...........FunctionSegment OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:10] ..........FunctionSegment Funct.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:10] ..........TableExpressionSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:12] ............TableReferenceSegment Delim._look_ahead_match IN [ls=1, seg='2'] [PD:0 MD:12] ............TableReferenceSegment Delim._look_ahead_match SI [mq=[], sb=['2']] [PD:0 MD:12] ............TableReferenceSegment OneOf.match PRN [ns=1, ps=10, ms=0, pruned=[<Ref: OnKeywordSegment>, <Ref: AsKeywordSegment>, <Ref: UsingKeywordSegment>, <Ref: CommaSegment>, <Ref: CastOperatorSegment>, <Ref: StartSquareBracketSegment>, <Ref: StartBracketSegment>, <Ref: ColonSegment>, <Ref: DelimiterSegment>, <class 'sqlfluff.core.parser.segments.base.BracketedSegment'>], opts=[<Ref: BinaryOperatorGrammar>]] [PD:0 MD:13] .............BinaryOperatorGrammar OneOf.match PRN [ns=1, ps=3, ms=0, pruned=[<Ref: ArithmeticBinaryOperatorGrammar>, <Ref: StringBinaryOperatorGrammar>, <Ref: BooleanBinaryOperatorGrammar>], opts=[<Ref: ComparisonOperatorGrammar>]] [PD:0 MD:14] ..............ComparisonOperatorGrammar OneOf.match PRN [ns=1, ps=9, ms=0, pruned=[<Ref: EqualsSegment>, <Ref: EqualsSegment_a>, <Ref: EqualsSegment_b>, <Ref: GreaterThanSegment>, <Ref: LessThanSegment>, <Ref: GreaterThanOrEqualToSegment>, <Ref: LessThanOrEqualToSegment>, <Ref: NotEqualToSegment_a>, <Ref: NotEqualToSegment_b>], opts=[<Ref: LikeOperatorSegment>]] [PD:0 MD:15] ...............ComparisonOperatorGrammar Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:14] ..............ComparisonOperatorGrammar OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:14] ..............BinaryOperatorGrammar Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:13] .............BinaryOperatorGrammar OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:13] .............TableReferenceSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:12] ............TableReferenceSegment OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:12] ............TableReferenceSegment NonCo.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:12] ............SingleIdentifierGrammar OneOf.match PRN [ns=2, ps=0, ms=0, pruned=[], opts=[<Ref: NakedIdentifierSegment>, <Ref: QuotedIdentifierSegment>]] [PD:0 MD:13] .............SingleIdentifierGrammar Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:13] .............SingleIdentifierGrammar Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:12] ............SingleIdentifierGrammar OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:12] ............TableReferenceSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:11] ...........TableReferenceSegment Delim.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:10] ..........TableReferenceSegment Table.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:10] ..........TableExpressionSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:9 ] .........TableExpressionSegment OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:8 ] ........TableExpressionSegment Table.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:8 ] ........FromExpressionElementSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:7 ] .......FromExpressionElementSegment Optio.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:6 ] ......FromExpressionElementSegment Seque.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:5 ] .....FromExpressionElementSegment FromE.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:5 ] .....FromExpressionSegment Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:4 ] ....FromExpressionSegment OneOf.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:3 ] ...FromExpressionSegment Seque.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:2 ] ..FromExpressionSegment FromE.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:2 ] ..None Ref.match OUT [match=<MatchResult 0/1: ''>, seg="'2'"] [PD:0 MD:1 ] .None Delim.match OUT [match=<MatchResult 0/2: ''>, seg="'values 1 , 2'"] [PD:0 MD:0 ] None Seque.match OUT [match=<MatchResult 0/9: ''>, seg="'from values 1 , 2'"]

Nothing I'm aware of. @alanmcruickshank wrote the parser; maybe he has something to add?

test/fixtures/dialects/ansi/select_from_values.sql

…park3, per suggestion: sqlfluff#2245 (comment)

test/fixtures/rules/std_rule_cases/L025.yml

mcannamela · 2022-01-26T13:52:00Z

@barrywhart I've copied tests from your PR as you suggest

@jpy-git your PR allowed me to reuse the ValuesClauseSegment from ansi, which is great

I'm going to take it out of draft, ought not be more than a couple small items outstanding.

barrywhart · 2022-01-26T17:48:31Z

@mcannamela: A couple of the test suites have failures. One is the normal Python tests.

Another is a new test added yesterday which runs the full sqlfluff lint on each of the parse test files in test/fixtures/dialects/spark3 and the other sibling directories. This tends to catch two kinds of issues:

Latent rule bugs
Dialects that may do some things in a new or different way compared to other dialects, causing rules to fail.

Can you look at the first? I'll take an initial look at the second one. Many rules are failing, but the error message is the same for each, and I think several of them reuse some of the same underlying components, so it may be essentially the same error repeated multiple times.

barrywhart · 2022-01-26T18:00:15Z

I have some possible fixes for the rule issues. I'll create a PR against your PR later today with those fixes.

tunetheweb · 2022-01-26T18:03:30Z

I have some possible fixes for the rule issues. I'll create a PR against your PR later today with those fixes.

Would a separate PR be better? Can then merge main into this branch after?

barrywhart · 2022-01-26T18:26:24Z

@tunetheweb: Good idea! I created it as a standalone PR. It's similar to some of your fixes yesterday. #2488

src/sqlfluff/dialects/dialect_spark3.py

…into mc-fix-spark-values

mcannamela · 2022-01-31T18:38:27Z

Thanks everybody for reviewing; I think I have addressed all feedback so assuming checks pass I'm hunting for ✅ s

tunetheweb

OK think this is good to merge. Thanks for all your work on it. Realise there's still more to do, but had a quick look and can't see a simple solution so let's merge as is and tackle later.

One suggestion to correct docstring back ticks that I'll merge in.

src/sqlfluff/dialects/dialect_spark3.py

mcannamela added 19 commits December 21, 2021 08:52

Examples of VALUES failing to parse

2b64b2b

Merge branch 'main' into mc-fix-spark-values

3674e97

Additional example

3106c47

Try VALUES before functions in TableExpressionSegment

7607a1c

show that VALUES clauses themselves all parse

9d627a5

even simpler multi-value test

5ec37a1

yamls

c34ba3a

even simpler test queries

00d8421

even simpler test queries

2ad151f

type annotations

6d5d5eb

even more tests around values

8a4c954

very explicit grammar tests

8100d44

fragments don't work in fixtured tests

2331c28

show that it matches lexed segments as well

5d1866d

works with the values clause bracketed

3797e18

also one column cases can fail

554b35a

refresh yamls, consolidate working cases to one fixture to see where …

ce5c010

…we are

refine the grammar test a bit

ac64e67

Slight cleanup of the new grammar pieces

186fa7a