[SPARK-12687][SQL] Support from clause surrounded by ().
#10660
Conversation
There was a problem hiding this comment.
Is it possible to test fix by adding these to the parser unit test without running the queries?
As we develop more and more Spark features, I'd like to encourage doing actual unit tests or integration tests that run the minimal required code.
There was a problem hiding this comment.
+1, these could be added into CatalystQlSuite
There was a problem hiding this comment.
ok. I added it here in order to make sure the results are correct. I will move it to CatalystQlSuite.
|
cc @davies |
There was a problem hiding this comment.
How does the order matter? Don't really understand ANTLR3, thanks!
There was a problem hiding this comment.
Yes, it matters. Without changing the order, select * from ((select 1 as a) union (select 1 as a)) t doesn't work because ((select 1 as a) union will be parsed as LPAREN joinSource RPAREN first and the parser will complain wrong input '(' expecting ) near union.
|
Test build #49009 has finished for PR 10660 at commit
|
There was a problem hiding this comment.
Can't we nest this rule? It seems a bit redundant to repeat the match rule twice.
There was a problem hiding this comment.
Because in the following we need to access some of the sub clauses such as selectClause, clusterByClause, etc., with the syntax seems we can't simply extract the common part as a single rule.
There was a problem hiding this comment.
Yeah, just tried this. My suggestion does not due to the set handling.
|
LGTM |
|
Merging this into master, thanks! |
5 participants
JIRA: https://issues.apache.org/jira/browse/SPARK-12687
Some queries such as
(select 1 as a) union (select 2 as a)can't work. This patch fixes it.