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

[SPARK-24658][SQL] Remove workaround for ANTLR bug #21641

Closed
wants to merge 1 commit into from
Closed

[SPARK-24658][SQL] Remove workaround for ANTLR bug #21641

wants to merge 1 commit into from

Conversation

wangyum
Copy link
Member

@wangyum wangyum commented Jun 26, 2018

What changes were proposed in this pull request?

Issue antlr/antlr4#781 has already been fixed, so the workaround of extracting the pattern into a separate rule is no longer needed. The presto already removed it: prestodb/presto#10744.

How was this patch tested?

Existing tests

@maropu
Copy link
Member

maropu commented Jun 26, 2018

I'm not 100% that we could remove this workaround because we hold some workarounds that are not essentially needed anymore, e.g., SnappyOutputStreamWrapper.

Anyway, in the description above, can you put the antlr version that have fixed the issue?

@SparkQA
Copy link

SparkQA commented Jun 26, 2018

Test build #92324 has finished for PR 21641 at commit e9a1b2c.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member

maropu commented Jun 26, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Jun 26, 2018

Test build #92327 has finished for PR 21641 at commit e9a1b2c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member

Based on my understanding, the issue should be fixed by antlr/antlr4#873 , which is released in https://github.com/antlr/antlr4/releases/tag/4.5.1

Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks! Merged to master

@asfgit asfgit closed this in dcaa49f Jun 26, 2018
@wangyum wangyum deleted the ANTLR-780 branch June 27, 2018 08:11
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.

4 participants