[SPARK-6202] [SQL] enable variable substitution on test framework #4930
Conversation
|
Test build #28333 has started for PR 4930 at commit
|
|
Test build #28333 has finished for PR 4930 at commit
|
|
Test PASSed. |
|
LGTM |
|
Can you explain the reason that you need to do the change when we already have substitution in |
|
Also, add a comment in the code to explain the reason will be great. |
|
we should substitute variables in hql to pass the text to parseSql() as a parameter. Hive parser need substituted text. HiveContext.sql() does this but return a DataFrame, while we need a logicalPlan so we cannot reuse that. |
|
Test build #28775 has started for PR 4930 at commit
|
| // we should substitute variables in hql to pass the text to parseSql() as a parameter. | ||
| // Hive parser need substituted text. HiveContext.sql() does this but return a DataFrame, | ||
| // while we need a logicalPlan so we cannot reuse that. | ||
| protected[hive] class SubstitutedHiveQLQueryExecution(hql: String) | ||
| extends this.QueryExecution(HiveQl.parseSql(hql)) { |
There was a problem hiding this comment.
Can you just change this line and do what we have in sql (new VariableSubstitution().substitute(hiveconf, hql))?
There was a problem hiding this comment.
but runSqlHive() also require a substituted text for use.
|
Test build #28775 has finished for PR 4930 at commit
|
|
Test PASSed. |
| // we should substitute variables in hql to pass the text to parseSql() as a parameter. | ||
| // Hive parser need substituted text. HiveContext.sql() does this but return a DataFrame, | ||
| // while we need a logicalPlan so we cannot reuse that. | ||
| protected[hive] class SubstitutedHiveQLQueryExecution(hql: String) | ||
| extends this.QueryExecution(HiveQl.parseSql(hql)) { | ||
| def hiveExec() = runSqlHive(hql) |
There was a problem hiding this comment.
@yhuai runSqlHive calls Driver.run directly without do substitution, so we have to substitute for this and QueryExecution both.
There was a problem hiding this comment.
Seems eventually Driver.run will call Driver.compile, which will do the substitution?
There was a problem hiding this comment.
@adrian-wang how about adding the substitution in HiveContext.runSqlHive or HiveContext.runHive? Then we probably not necessary to change anything in TestHive.
|
Test build #28905 has started for PR 4930 at commit
|
|
Test build #28905 has finished for PR 4930 at commit
|
|
Test PASSed. |
|
ping @yhuai |
|
Test build #28981 has started for PR 4930 at commit
|
|
Test build #28982 has started for PR 4930 at commit
|
|
Test build #28981 has finished for PR 4930 at commit
|
|
Test PASSed. |
|
Test build #28982 has finished for PR 4930 at commit
|
|
Test PASSed. |
|
LGTM. Is there any Hive query test that we should enable? |
|
There is one of "mapjoin_addjar", and I'll add that after I refactor #4586 . So I'd like this one merge first, thanks a lot! |
|
Thanks! Merged to master. |
No description provided.