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
[SPARK-15279][SQL] Catch conflicting SerDe when creating table #13068
Conversation
Test build #58425 has finished for PR 13068 at commit
|
2e6fa67
to
4ffaf59
Compare
Test build #58428 has finished for PR 13068 at commit
|
Test build #58430 has finished for PR 13068 at commit
|
retest this please |
Test build #58444 has finished for PR 13068 at commit
|
(rowFormatCtx, createFileFormatCtx.fileFormat) match { | ||
case (_, ffTable: TableFileFormatContext) => | ||
if (visitTableFileFormat(ffTable).serde.isDefined) { | ||
throw operationNotAllowed(s"ROW FORMAT is not compatible with $cff", parentCtx) |
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.
Maybe we can be explicit that SERDE
is not allowed for this case? btw, should we remove SERDE
part from INPUTFORMAT inFmt=STRING OUTPUTFORMAT outFmt=STRING (SERDE serdeCls=STRING)?
in SqlBase.g4
?
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.
yeah, I like that actually! Hive doesn't support STORED AS serde too: https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL. I actually have no idea why we accept it.
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.
Why don't we support this? What's the problem? Can you give me an example? thx
All comments have been addressed |
Thanks! LGTM |
Test build #59027 has finished for PR 13068 at commit
|
Test build #59034 has finished for PR 13068 at commit
|
retest this please |
Test build #59042 has finished for PR 13068 at commit
|
m2.0 |
## What changes were proposed in this pull request? The user may do something like: ``` CREATE TABLE my_tab ROW FORMAT SERDE 'anything' STORED AS PARQUET CREATE TABLE my_tab ROW FORMAT SERDE 'anything' STORED AS ... SERDE 'myserde' CREATE TABLE my_tab ROW FORMAT DELIMITED ... STORED AS ORC CREATE TABLE my_tab ROW FORMAT DELIMITED ... STORED AS ... SERDE 'myserde' ``` None of these should be allowed because the SerDe's conflict. As of this patch: - `ROW FORMAT DELIMITED` is only compatible with `TEXTFILE` - `ROW FORMAT SERDE` is only compatible with `TEXTFILE`, `RCFILE` and `SEQUENCEFILE` ## How was this patch tested? New tests in `DDLCommandSuite`. Author: Andrew Or <andrew@databricks.com> Closes #13068 from andrewor14/row-format-conflict. (cherry picked from commit 2585d2b) Signed-off-by: Andrew Or <andrew@databricks.com>
What changes were proposed in this pull request?
The user may do something like:
None of these should be allowed because the SerDe's conflict. As of this patch:
ROW FORMAT DELIMITED
is only compatible withTEXTFILE
ROW FORMAT SERDE
is only compatible withTEXTFILE
,RCFILE
andSEQUENCEFILE
How was this patch tested?
New tests in
DDLCommandSuite
.