Skip to content

Commit

Permalink
[HOTFIX] Fix SDV test case failure after PR #2645
Browse files Browse the repository at this point in the history
**Changes:

1. #2645 has blocked the schema for external table. But SDV test cases were not updated. Hence updated the test cases
2. table properties was not blocked for external table, so even when a valid table property is passed, validation of table properties will fail for external table, as fields (schema) will be null.
Hence blocked this with proper error message**

This closes #2809
  • Loading branch information
ajantha-bhat authored and jackylk committed Oct 22, 2018
1 parent a9e405f commit 8c49e5b
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 18 deletions.
Expand Up @@ -21,7 +21,7 @@ package org.apache.carbondata.cluster.sdv.generated
import java.io.{ByteArrayInputStream, ByteArrayOutputStream, DataInputStream, InputStream}
import java.util

import org.apache.spark.sql.Row
import org.apache.spark.sql.{AnalysisException, Row}
import org.apache.spark.sql.common.util.QueryTest
import org.scalatest.BeforeAndAfterEach
import scala.collection.JavaConverters._
Expand Down Expand Up @@ -194,7 +194,7 @@ class SDKwriterTestCase extends QueryTest with BeforeAndAfterEach {
sql("DROP TABLE IF EXISTS sdkTable")

sql(
s"""CREATE EXTERNAL TABLE sdkTable(name string,age int) STORED BY
s"""CREATE EXTERNAL TABLE sdkTable STORED BY
|'carbondata' LOCATION
|'$writerPath' """.stripMargin)
checkAnswer(sql("select * from sdkTable"), Seq(Row("abc0", 0, 0.0),
Expand Down Expand Up @@ -228,7 +228,7 @@ class SDKwriterTestCase extends QueryTest with BeforeAndAfterEach {
sql("DROP TABLE IF EXISTS sdkTable")

sql(
s"""CREATE EXTERNAL TABLE sdkTable(name string,age int) STORED BY
s"""CREATE EXTERNAL TABLE sdkTable STORED BY
|'carbondata' LOCATION
|'$writerPath' """.stripMargin)

Expand Down Expand Up @@ -266,26 +266,25 @@ class SDKwriterTestCase extends QueryTest with BeforeAndAfterEach {
sql("DROP TABLE IF EXISTS sdkTable1")
sql("DROP TABLE IF EXISTS sdkTable2")
sql(
s"""CREATE EXTERNAL TABLE sdkTable1(name string,age int) STORED BY
s"""CREATE EXTERNAL TABLE sdkTable1 STORED BY
|'carbondata' LOCATION
|'$writerPath' """.stripMargin)
sql(
s"""CREATE EXTERNAL TABLE sdkTable2(name string,age int) STORED BY
s"""CREATE EXTERNAL TABLE sdkTable2 STORED BY
|'carbondata' LOCATION
|'$writerPath' """.stripMargin)

sql("insert into sdkTable1 select *from sdkTable2")
checkAnswer(sql("select count(*) from sdkTable1"), Seq(Row(6)))
}

test("test create External Table with Schema with partition, external table should " +
"ignore schema and partition") {
test("test create External Table without Schema") {
buildTestDataSingleFile()
assert(FileFactory.getCarbonFile(writerPath).exists())
sql("DROP TABLE IF EXISTS sdkTable")

sql(
s"""CREATE EXTERNAL TABLE sdkTable(name string) PARTITIONED BY (age int) STORED BY
s"""CREATE EXTERNAL TABLE sdkTable STORED BY
|'carbondata' LOCATION
|'$writerPath' """.stripMargin)

Expand All @@ -301,7 +300,7 @@ class SDKwriterTestCase extends QueryTest with BeforeAndAfterEach {
sql("DROP TABLE IF EXISTS table1")

sql(
s"""CREATE EXTERNAL TABLE sdkTable(name string) PARTITIONED BY (age int) STORED BY
s"""CREATE EXTERNAL TABLE sdkTable STORED BY
|'carbondata' LOCATION
|'$writerPath' """.stripMargin)

Expand All @@ -323,17 +322,18 @@ class SDKwriterTestCase extends QueryTest with BeforeAndAfterEach {
Seq(Row(0)))
}

test("test create External Table with Table properties should ignore tblproperties") {
test("test create External Table with Table properties should fail") {
buildTestDataSingleFile()
assert(FileFactory.getCarbonFile(writerPath).exists())
sql("DROP TABLE IF EXISTS sdkTable")

sql(
s"""CREATE EXTERNAL TABLE sdkTable(name string,age int) STORED BY
|'carbondata' LOCATION
|'$writerPath' TBLPROPERTIES('sort_scope'='batch_sort') """.stripMargin)

checkExistence(sql("Describe formatted sdkTable "), false, "batch_sort")
val ex = intercept[AnalysisException] {
sql(
s"""CREATE EXTERNAL TABLE sdkTable STORED BY
|'carbondata' LOCATION
|'$writerPath' TBLPROPERTIES('sort_scope'='batch_sort') """.stripMargin)
}
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")
Expand Down
Expand Up @@ -101,7 +101,7 @@ class TestCreateExternalTable extends QueryTest with BeforeAndAfterAll {
|LOCATION '$storeLocation/origin'
""".stripMargin)
}
assert(ex.message.contains("Schema may not be specified for external table"))
assert(ex.message.contains("Schema must not be specified for external table"))

sql("DROP TABLE IF EXISTS source")

Expand Down
Expand Up @@ -123,6 +123,13 @@ object CarbonSparkSqlParserUtil {
if (partitionFields.nonEmpty && options.isStreaming) {
operationNotAllowed("Streaming is not allowed on partitioned table", partitionColumns)
}

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)
}

// validate tblProperties
val bucketFields = parser.getBucketFields(tableProperties, fields, options)
var isTransactionalTable: Boolean = true
Expand All @@ -132,7 +139,7 @@ object CarbonSparkSqlParserUtil {
// user provided schema for this external table, this is not allow currently
// see CARBONDATA-2866
operationNotAllowed(
"Schema may not be specified for external table", columns)
"Schema must not be specified for external table", columns)
}
if (partitionByStructFields.nonEmpty) {
operationNotAllowed(
Expand Down

0 comments on commit 8c49e5b

Please sign in to comment.