[GitHub] [spark] sunchao commented on a change in pull request #34148: [SPARK-36895][SQL] Add Create Index syntax support

[GitBox <gi...@apache.org>]

sunchao commented on a change in pull request #34148:
URL: https://github.com/apache/spark/pull/34148#discussion_r720709548



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -425,4 +425,27 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
       assert(m.contains("\"TABLEENGINENAME\" not found"))
     }
   }
+
+  test("CREATE INDEX") {
+    withTable("h2.test.new_table") {
+      sql("CREATE TABLE h2.test.new_table(col1 INT, col2 STRING)")
+      val e1 = intercept[SQLFeatureNotSupportedException] {
+        sql("CREATE index i1 ON h2.test.new_table (col1)")
+      }.getMessage
+      assert(e1.contains("IndexName i1"))
+      assert(e1.contains("indexType "))
+      assert(e1.contains("columns ArrayBuffer(col1)"))
+
+      val e2 = intercept[SQLFeatureNotSupportedException] {

Review comment:
       can we test more combinations? e.g., when there is no option given to a column, or no option is given to the index, or an index type is specified etc.

##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -1780,6 +1795,7 @@ WINDOW: 'WINDOW';
 WITH: 'WITH';
 YEAR: 'YEAR';
 ZONE: 'ZONE';
+

Review comment:
       nit: unrelated change

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveCatalogs.scala
##########
@@ -88,6 +88,17 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
         val namespace = if (ns.nonEmpty) Some(ns) else None
         SetCatalogAndNamespace(catalogManager, Some(catalog.name()), namespace)
       }
+
+    case c @ CreateIndex(UnresolvedDBObjectName(

Review comment:
       why do we need to use `UnresolvedDBObjectName` here?

##########
File path: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4
##########
@@ -1034,6 +1045,10 @@ alterColumnAction
     | setOrDrop=(SET | DROP) NOT NULL
     ;
 
+indexPropertyList

Review comment:
       can we combine this with `tablePropertyList`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org