-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-14396] [SQL] Throw Exceptions for DDLs of Partitioned Views #12169
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
| | ALTER TABLE tableIdentifier | ||
| SET SKEWED LOCATION skewedLocationList #setTableSkewLocations | ||
| | ALTER TABLE tableIdentifier ADD (IF NOT EXISTS)? | ||
| | ALTER kind=TABLE tableIdentifier ADD (IF NOT EXISTS)? |
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.
Minor: we don't really need kind. We can also just check for the existance of the VIEW keyword, e.g.: ctx.VIEW != null.
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.
Thanks! Will do.
|
Test build #54959 has finished for PR 12169 at commit
|
# Conflicts: # sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveDDLCommandSuite.scala
|
Test build #54983 has finished for PR 12169 at commit
|
|
Thanks for the review! @hvanhovell Also cc @yhuai @andrewor14 |
| */ | ||
| override def visitAddTablePartition( | ||
| ctx: AddTablePartitionContext): LogicalPlan = withOrigin(ctx) { | ||
| if (ctx.VIEW != null) throw new ParseException(s"Unsupported partitioned view", ctx) |
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 think @yhuai prefers to say something like Operation not allowed: partitioned views for the message (here and everywhere else)
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.
Will do it now. Thanks!
|
Looks pretty good, just a minor comment. |
|
retest this please |
|
Test build #55031 has finished for PR 12169 at commit
|
|
Build is broken I think. retest this please |
| * }}} | ||
| */ | ||
| override def visitCreateView(ctx: CreateViewContext): LogicalPlan = withOrigin(ctx) { | ||
| // Pass a partitioned view on to hive. |
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.
this needs to be deleted
|
LGTM we just need to make tests pass now |
|
Test build #55049 has finished for PR 12169 at commit
|
|
Test build #55072 has finished for PR 12169 at commit
|
|
Merged into master. |
|
@gatorsmile Seems scala 2.10 build is broken. Can you take a look? |
|
Sure. Could you show me how to find the build log? |
|
Or I need to do it in my local environment? |
|
I see the problem. |
|
Will fix it using a PR with [HOT]. Sorry for it. |
What changes were proposed in this pull request?
Because the concept of partitioning is associated with physical tables, we disable all the supports of partitioned views, which are defined in the following three commands in Hive DDL Manual:
An exception is thrown when users issue any of these three DDL commands.
How was this patch tested?
Added test cases for parsing create view and changed the existing test cases to verify if the exceptions are thrown.