[SPARK-26905][SQL] Follow the SQL:2016 reserved keywords #28807

maropu · 2020-06-12T06:46:29Z

What changes were proposed in this pull request?

This PR intends to move keywords ANTI, SEMI, and MINUS from reserved to non-reserved.

Why are the changes needed?

To comply with the ANSI/SQL standard.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added tests.

SparkQA · 2020-06-12T07:05:02Z

Test build #123894 has finished for PR 28807 at commit 17b8f2c.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-06-12T07:07:10Z

retest this please

SparkQA · 2020-06-12T13:17:18Z

Test build #123896 has finished for PR 28807 at commit 17b8f2c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-06-12T13:33:56Z

retest this please

SparkQA · 2020-06-12T14:29:02Z

Test build #123929 has finished for PR 28807 at commit 17b8f2c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-06-12T22:03:42Z

retest this please

SparkQA · 2020-06-13T02:09:27Z

Test build #123946 has finished for PR 28807 at commit f8cbbb1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-06-13T02:50:45Z

...atalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala

+      Files.copy(is, tmpFile.toPath)
+      val reservedKeywordsInSql2016 = Files.readAllLines(tmpFile.toPath)
+        .asScala.filterNot(_.startsWith("--")).map(_.trim).toSet
+      assert(((reservedKeywordsInAnsiMode -- Set("!")) -- reservedKeywordsInSql2016).isEmpty)


I noticed that, since NOT reserved, ! also reserved, too. (NOT: 'NOT' | '!';);

scala> sql("SET spark.sql.ansi.enabled=false") scala> spark.sql("create table r2 (! int);") res2: org.apache.spark.sql.DataFrame = [] scala> sql("SET spark.sql.ansi.enabled=true") scala> spark.sql("create table r2 (! int);") org.apache.spark.sql.catalyst.parser.ParseException: no viable alternative at input '!'(line 1, pos 17) == SQL == create table r2 (! int); -----------------^^^

@cloud-fan Is this expected? FYI: It seems PostgreSQL cannot accept ! in column names;

postgres=# create table r2 (! int); 2020-06-13 11:30:24.495 JST [36406] ERROR: syntax error at or near "!" at character 18 2020-06-13 11:30:24.495 JST [36406] STATEMENT: create table r2 (! int); ERROR: syntax error at or near "!" LINE 1: create table r2 (! int);

I don't think it's intentional, but we can't break it now. If it's forbidden under ansi mode, I think it's good enough?

SparkQA · 2020-06-13T03:50:42Z

Test build #123944 has finished for PR 28807 at commit 17b8f2c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-06-13T07:05:02Z

Test build #123959 has finished for PR 28807 at commit 086a2ba.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-06-13T07:21:22Z

retest this please

SparkQA · 2020-06-13T12:24:00Z

Test build #123971 has finished for PR 28807 at commit 086a2ba.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-06-15T02:33:19Z

...atalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala

        s"${reservedKeywordsInAnsiMode.size} found.")
  }

+  test("should follow reserved keywords in SQL:2016") {


can we make the test name clear? What we are testing is: reserved keywords in Spark are also reserved in SQL 2016

Looks clearer, okay, I'll update. Thanks!

cloud-fan · 2020-06-15T02:34:25Z

...atalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala

+      Files.copy(is, tmpFile.toPath)
+      val reservedKeywordsInSql2016 = Files.readAllLines(tmpFile.toPath)
+        .asScala.filterNot(_.startsWith("--")).map(_.trim).toSet
+      assert(((reservedKeywordsInAnsiMode -- Set("!")) -- reservedKeywordsInSql2016).isEmpty)


I think symbol is not a keyword. Can we update symbolsToExpandIntoDifferentLiterals?

Yea, will do.

maropu · 2020-06-15T04:44:57Z

...atalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/TableIdentifierParserSuite.scala


  test("check # of reserved keywords") {
-    val numReservedKeywords = 78
+    val numReservedKeywords = 74


Note: ANTI, SEMI, MINUS, and ! are removed.

SparkQA · 2020-06-15T07:05:02Z

Test build #124031 has finished for PR 28807 at commit eeceb30.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-06-15T07:51:39Z

retest this please

SparkQA · 2020-06-15T13:36:51Z

Test build #124041 has finished for PR 28807 at commit eeceb30.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-06-15T15:30:13Z

Thanks! Merged to master.
Note that, since this fix conflicts with branch-3.0, I'll open another PR for that.

### What changes were proposed in this pull request? This PR intends to move keywords `ANTI`, `SEMI`, and `MINUS` from reserved to non-reserved. ### Why are the changes needed? To comply with the ANSI/SQL standard. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Added tests. Closes #28807 from maropu/SPARK-26905-2. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Takeshi Yamamuro <[email protected]>

maropu · 2020-06-15T15:49:39Z

Ah, the cause of the conflict is that I didn't merge #28802 into branch-3.0. Since #28802 is just for test improvements, I've cherry-picked this commit and #28802 together into branch-3.0. If any problem, please let me know.

dongjoon-hyun · 2020-06-16T15:45:23Z

No problem. It looks okay, @maropu .

maropu · 2020-06-16T15:47:41Z

Thanks for the check, @dongjoon-hyun .

probot-autolabeler bot added DOCS SQL labels Jun 12, 2020

maropu changed the title ~~[WIP][SPARK-26905][SQL][TESTS] Follow the SQL:2016 reserved keywords~~ [WIP][SPARK-26905][SQL] Follow the SQL:2016 reserved keywords Jun 12, 2020

Fix

f8cbbb1

maropu force-pushed the SPARK-26905-2 branch from 17b8f2c to f8cbbb1 Compare June 12, 2020 22:17

maropu added 2 commits June 13, 2020 11:36

fix

39cfc6f

'\!' reserved?

086a2ba

maropu commented Jun 13, 2020

View reviewed changes

maropu changed the title ~~[WIP][SPARK-26905][SQL] Follow the SQL:2016 reserved keywords~~ [SPARK-26905][SQL] Follow the SQL:2016 reserved keywords Jun 15, 2020

cloud-fan reviewed Jun 15, 2020

View reviewed changes

review

eeceb30

maropu commented Jun 15, 2020

View reviewed changes

cloud-fan approved these changes Jun 15, 2020

View reviewed changes

maropu closed this in 3698a14 Jun 15, 2020

[SPARK-26905][SQL] Follow the SQL:2016 reserved keywords #28807

[SPARK-26905][SQL] Follow the SQL:2016 reserved keywords #28807

Uh oh!

Conversation

maropu commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Jun 12, 2020

Uh oh!

maropu commented Jun 12, 2020

Uh oh!

SparkQA commented Jun 12, 2020

Uh oh!

maropu commented Jun 12, 2020

Uh oh!

SparkQA commented Jun 12, 2020

Uh oh!

maropu commented Jun 12, 2020

Uh oh!

SparkQA commented Jun 13, 2020

Uh oh!

maropu Jun 13, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 13, 2020

Uh oh!

SparkQA commented Jun 13, 2020

Uh oh!

maropu commented Jun 13, 2020

Uh oh!

SparkQA commented Jun 13, 2020

Uh oh!

cloud-fan Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

maropu Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

maropu Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

maropu Jun 15, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 15, 2020

Uh oh!

cloud-fan commented Jun 15, 2020

Uh oh!

SparkQA commented Jun 15, 2020

Uh oh!

maropu commented Jun 15, 2020

Uh oh!

maropu commented Jun 15, 2020

Uh oh!

dongjoon-hyun commented Jun 16, 2020

Uh oh!

maropu commented Jun 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maropu commented Jun 12, 2020 •

edited

Loading