-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-12982][SQL] Add table name validation in temp table registration #11051
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
Changes from all commits
290b86b
7c7b503
3407fd7
4c25f2e
a9e3b32
d576496
f3c7bbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1291,4 +1291,16 @@ class DataFrameSuite extends QueryTest with SharedSQLContext { | |
| Seq(1 -> "a").toDF("i", "j").filter($"i".cast(StringType) === "1"), | ||
| Row(1, "a")) | ||
| } | ||
|
|
||
| test("SPARK-12982: Add table name validation in temp table registration") { | ||
| val df = Seq("foo", "bar").map(Tuple1.apply).toDF("col") | ||
| // invalid table name test as below | ||
| intercept[AnalysisException](df.registerTempTable("t~")) | ||
| // valid table name test as below | ||
| df.registerTempTable("table1") | ||
| // another invalid table name test as below | ||
| intercept[AnalysisException](df.registerTempTable("#$@sum")) | ||
| // another invalid table name test as below | ||
| intercept[AnalysisException](df.registerTempTable("table!#")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmmm... this one should fail. Let me submit a PR for this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a new bug ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a bug, but it was in code base already. The fix is trivial (I need to add a separate rule for parsing table identifiers - which checks for an EOF).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok thanks for the info. Let me know if you need any help from me. |
||
| } | ||
| } | ||
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.
Could you also add a few more different valid & invalid table names?
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.
yes @hvanhovell
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.
ping
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.
@hvanhovell @davies
Shall I commit the changes for the above review comments now ?