-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-14211][SQL] Remove ANTLR3 based parser #12071
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
Conversation
| (AS? query)? #createTable | ||
| | ANALYZE TABLE tableIdentifier partitionSpec? COMPUTE STATISTICS | ||
| (identifier | FOR COLUMNS identifierSeq?) #analyze | ||
| (identifier | FOR COLUMNS identifierSeq?)? #analyze |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not related right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to update the StatisticsSuite and change its parser to the HiveSqlParser & this error popped up. I could also do this in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's fine, just confirming
|
LGTM, can't wait to merge this. |
| "TOK_SHOW_SET_ROLE" -> "SHOW CURRENT ROLES / SET ROLE", | ||
| "TOK_SHOW_TRANSACTIONS" -> "SHOW TRANSACTIONS", | ||
| "TOK_SHOWINDEXES" -> "SHOW INDEXES", | ||
| "TOK_SHOWLOCKS" -> "SHOW LOCKS") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, what happens now if I run one of these? What exception will we get?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will produce the same error message, with the same nice message. See: https://github.com/hvanhovell/spark/blob/SPARK-14211/sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala#L84-L93
|
Test build #54550 has finished for PR 12071 at commit
|
|
Test build #2713 has finished for PR 12071 at commit
|
|
Test build #54610 has finished for PR 12071 at commit
|
|
retest this please |
|
Test build #54619 has finished for PR 12071 at commit
|
|
Test build #54624 has finished for PR 12071 at commit
|
|
Thanks - merging in master. |
What changes were proposed in this pull request?
This PR removes the ANTLR3 based parser, and moves the new ANTLR4 based parser into the
org.apache.spark.sql.catalyst.parser package.How was this patch tested?
Existing unit tests.
cc @rxin @andrewor14 @yhuai