[SPARK-19598][SQL]Remove the alias parameter in UnresolvedRelation #16956
Conversation
| @@ -54,10 +54,6 @@ object ResolveHints { | |||
|
|
|||
| val newNode = CurrentOrigin.withOrigin(plan.origin) { | |||
| plan match { | |||
| case r: UnresolvedRelation => | |||
There was a problem hiding this comment.
actually you can't remove this case entirely. you still need to match on UnresolvedRelation, but just use r.tableIdentifier.table
There was a problem hiding this comment.
oh, yes, remove the case will miss the hint on table without alias
|
Test build #72995 has finished for PR 16956 at commit
|
|
Test build #72996 has finished for PR 16956 at commit
|
|
Test build #72999 has finished for PR 16956 at commit
|
|
I like this change. To run unit test cases in your local environment, below are some common commands when I develop Spark SQL
|
| val table = UnresolvedRelation( | ||
| visitTableIdentifier(ctx.tableIdentifier), | ||
| Option(ctx.strictIdentifier).map(_.getText)) | ||
| table.optionalMap(ctx.sample)(withSample) |
There was a problem hiding this comment.
This should be kept in the last line
There was a problem hiding this comment.
yes, I can not modify the original default expression order between alias and and sample.
|
@gatorsmile thanks for your good suggestion～Usually I run the single test case in IDEA，and let jenkins run testcases globaly，I will change the bad behavior， thanks a lot! |
|
Test build #73044 has started for PR 16956 at commit |
|
retest this please |
|
Test build #73045 has finished for PR 16956 at commit
|
|
@rxin @gatorsmile help to continue to review this? Thanks :) |
|
LGTM |
|
retest this please |
|
Test build #73129 has finished for PR 16956 at commit
|
|
Thanks! Merging to master. |
## What changes were proposed in this pull request? Remove the alias parameter in `UnresolvedRelation`, and use `SubqueryAlias` to replace it. This can simplify some `match case` situations. For example, the broadcast hint pull request can have one fewer case https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveHints.scala#L57-L61 ## How was this patch tested? add some unit tests Author: windpiger <songjun@outlook.com> Closes apache#16956 from windpiger/removeUnresolveTableAlias.
What changes were proposed in this pull request?
Remove the alias parameter in
UnresolvedRelation, and useSubqueryAliasto replace it.This can simplify some
match casesituations.For example, the broadcast hint pull request can have one fewer case https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveHints.scala#L57-L61
How was this patch tested?
add some unit tests