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

Correct the two-stage parsing strategy of antlr parser #1800

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cfmcgrady
Copy link
Contributor

Description

to fix #1205 and revert the changes in #1206

As mentioned in antlr/antlr4#192 (comment)

You can save a great deal of time on correct inputs by using a two-stage parsing strategy.

  1. Attempt to parse the input using BailErrorStrategy and PredictionMode.SLL. If no exception is thrown, you know the answer is correct.
  2. If a ParseCancellationException is thrown, retry the parse using the default settings (DefaultErrorStrategy and PredictionMode.LL).

Delta's antlr parser code is derived from Spark, and Spark's one is derived from Presto, the original implementation in Presto is wrong.

It was identified on Spark and got fixed in SPARK-42552 apache/spark#40835

How was this patch tested?

existing UT

Does this PR introduce any user-facing changes?

No

@cfmcgrady
Copy link
Contributor Author

cc @zsxwing

@tdas
Copy link
Contributor

tdas commented Nov 10, 2023

Hello @cfmcgrady sorry but this PR must have missed our attention. Could you update the PR with latest master. And is it possible to add an unit test? I see the Spark PR had a unit test.

@cfmcgrady
Copy link
Contributor Author

I think DeltaSqlParserSuite."OPTIMIZE command is parsed as expected" introduced in #1206 should cover this change. cc @tdas

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.

[BUG] ZORDER BY with where clause throws a ParseException
2 participants