[SPARK-12576][SQL] Enable expression parsing in CatalystQl #10649
Conversation
|
|
||
| /** Creates LogicalPlan for a given HiveQL string. */ | ||
| def createPlan(sql: String): LogicalPlan = { | ||
| protected def safeParse[T](sql: String, ast: ASTNode, toResult: ASTNode => T): T = { |
There was a problem hiding this comment.
use multi parameter list?
protected def safeParse[T](sql: String, ast: ASTNode)(toResult: ASTNode => T): Tthen you can call it in a slightly nicer way
safeParse(sql, ParseDriver.parseExpression(sql, conf)) { ast =>
...
}
|
Test build #48965 has finished for PR 10649 at commit
|
|
Test build #48983 has finished for PR 10649 at commit
|
|
Test build #49005 has finished for PR 10649 at commit
|
|
Test build #49022 has finished for PR 10649 at commit
|
| def parseExpression(sql: String): Expression = | ||
| safeParse(sql, ParseDriver.parseExpression(sql, conf))(selExprNodeToExpr(_).get) | ||
|
|
||
| /** Creates Expression for a given SQL string. */ |
There was a problem hiding this comment.
Expression -> TableIdentifier
|
|
||
| /** Creates LogicalPlan for a given HiveQL string. */ | ||
| def createPlan(sql: String): LogicalPlan = { | ||
| protected def safeParse[T](sql: String, ast: ASTNode)(toResult: ASTNode => T): T = { |
There was a problem hiding this comment.
there is a extra space before the def.
can you also add some doc explaining what this function does (e.g. what "safe" means)?
|
@cloud-fan can you also take a look? |
|
Test build #49184 has finished for PR 10649 at commit
|
|
I'm going to merge this. @cloud-fan it would be great for you to still take a look post-hoc and @hvanhovell can address feedback in his next pull request. Thanks. |
| assert(TableIdentifier("q", Some("d")) === parser.parseTableIdentifier("d.q")) | ||
| intercept[AnalysisException](parser.parseTableIdentifier("")) | ||
| // TODO parser swallows third identifier. | ||
| // intercept[AnalysisException](parser.parseTableIdentifier("d.q.g")) |
There was a problem hiding this comment.
are we going to support this?
There was a problem hiding this comment.
I don't think we should support this. Are there use cases for this? I'll create a fix, that'll throw an AnalysisException when we encounter this.
There was a problem hiding this comment.
yea, throw exception seems reasonable to me
…xpression string The parser currently parses the following strings without a hitch: * Table Identifier: * `a.b.c` should fail, but results in the following table identifier `a.b` * `table!#` should fail, but results in the following table identifier `table` * Expression * `1+2 r+e` should fail, but results in the following expression `1 + 2` This PR fixes this by adding terminated rules for both expression parsing and table identifier parsing. cc cloud-fan (we discussed this in #10649) jayadevanmurali (this causes your PR #11051 to fail) Author: Herman van Hovell <hvanhovell@questtec.nl> Closes #11159 from hvanhovell/SPARK-13276.
The PR allows us to use the new SQL parser to parse SQL expressions such as:
1 + sin(x*x)We enable this functionality in this PR, but we will not start using this actively yet. This will be done as soon as we have reached grammar parity with the existing parser stack.
cc @rxin