You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@carbondata.apache.org by ku...@apache.org on 2019/06/12 15:29:45 UTC

[carbondata] branch master updated: [CARBONDATA-3421] Fix create table without column with properties failed, but throw incorrect exception

This is an automated email from the ASF dual-hosted git repository.

kumarvishal09 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/carbondata.git


The following commit(s) were added to refs/heads/master by this push:
     new d2bc0a9  [CARBONDATA-3421] Fix create table without column with properties failed, but throw incorrect exception
d2bc0a9 is described below

commit d2bc0a9cf78b770f6507981e833799dcbfbb51d7
Author: jack86596 <ja...@gmail.com>
AuthorDate: Mon Jun 10 09:53:49 2019 +0800

    [CARBONDATA-3421] Fix create table without column with properties failed, but throw incorrect exception
    
    Problem:
    Create table without column with properties failed, but throw incorrect exception: Invalid table properties. The exception should be "create table without column."
    
    Solution:
    In CarbonSparkSqlParserUtil.createCarbonTable, we will do some validations like checking tblproperties, is column provided for external table so on.
     We can add one more validation here to check is column provided for normal table. If not, throw MalformedCarbonCommandException.
    
    This closes #3268
---
 .../cluster/sdv/generated/SDKwriterTestCase.scala          |  2 +-
 .../testsuite/createTable/TestCreateTableIfNotExists.scala |  6 ++++++
 .../org/apache/carbondata/spark/util/CommonUtil.scala      | 14 ++++++++------
 .../apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala |  6 ++++--
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/SDKwriterTestCase.scala b/integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/SDKwriterTestCase.scala
index 619bfb3..499c478 100644
--- a/integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/SDKwriterTestCase.scala
+++ b/integration/spark-common-cluster-test/src/test/scala/org/apache/carbondata/cluster/sdv/generated/SDKwriterTestCase.scala
@@ -333,7 +333,7 @@ class SDKwriterTestCase extends QueryTest with BeforeAndAfterEach {
            |'carbondata' LOCATION
            |'$writerPath' TBLPROPERTIES('sort_scope'='batch_sort') """.stripMargin)
     }
-    assert(ex.message.contains("table properties are not supported for external table"))
+    assert(ex.message.contains("Table properties are not supported for external table"))
   }
 
   test("Read sdk writer output file and test without carbondata and carbonindex files should fail")
diff --git a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableIfNotExists.scala b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableIfNotExists.scala
index b3fa0eb..35238dc 100644
--- a/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableIfNotExists.scala
+++ b/integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestCreateTableIfNotExists.scala
@@ -86,6 +86,12 @@ class TestCreateTableIfNotExists extends QueryTest with BeforeAndAfterAll {
     }
   }
 
+  test("test create table without column specified") {
+    val exception = intercept[MalformedCarbonCommandException] {
+      sql("create table TableWithoutColumn stored by 'carbondata' tblproperties('sort_columns'='')")
+    }
+    assert(exception.getMessage.contains("Creating table without column(s) is not supported"))
+  }
 
   override def afterAll {
     sql("use default")
diff --git a/integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala b/integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
index da42363..1c89a0c 100644
--- a/integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
+++ b/integration/spark-common/src/main/scala/org/apache/carbondata/spark/util/CommonUtil.scala
@@ -95,12 +95,14 @@ object CommonUtil {
 
   def validateTblProperties(tableProperties: Map[String, String], fields: Seq[Field]): Boolean = {
     var isValid: Boolean = true
-    tableProperties.foreach {
-      case (key, value) =>
-        if (!validateFields(key, fields)) {
-          isValid = false
-          throw new MalformedCarbonCommandException(s"Invalid table properties ${ key }")
-        }
+    if (fields.nonEmpty) {
+      tableProperties.foreach {
+        case (key, value) =>
+          if (!validateFields(key, fields)) {
+            isValid = false
+            throw new MalformedCarbonCommandException(s"Invalid table properties $key")
+          }
+      }
     }
     isValid
   }
diff --git a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala b/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala
index 46473f2..5c008f2 100644
--- a/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala
+++ b/integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParserUtil.scala
@@ -17,7 +17,6 @@
 
 package org.apache.spark.sql.parser
 
-import scala.collection.JavaConverters._
 import scala.collection.mutable
 
 import org.antlr.v4.runtime.tree.TerminalNode
@@ -124,10 +123,13 @@ object CarbonSparkSqlParserUtil {
       operationNotAllowed("Streaming is not allowed on partitioned table", partitionColumns)
     }
 
+    if (!external && fields.isEmpty) {
+      throw new MalformedCarbonCommandException("Creating table without column(s) is not supported")
+    }
     if (external && fields.isEmpty && tableProperties.nonEmpty) {
       // as fields are always zero for external table, cannot validate table properties.
       operationNotAllowed(
-        "table properties are not supported for external table", tablePropertyList)
+        "Table properties are not supported for external table", tablePropertyList)
     }
 
     // validate tblProperties