Skip to content

[SPARK-20680][SQL] Spark-sql do not support for creating table with void column datatype #28833

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.

Sign up for GitHub

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

Closed
LantaoJin wants to merge 12 commits into from
Closed

[SPARK-20680][SQL] Spark-sql do not support for creating table with void column datatype #28833

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fda353f
[SPARK-20680][SQL] Spark-sql do not support for void column datatype …
LantaoJin Jun 3, 2017
1e86674
[SPARK-20680][SQL] re-generate golden files
LantaoJin Jun 6, 2017
3b8ddec
Merge remote-tracking branch 'upstream/master' into SPARK-20680
LantaoJin Jun 15, 2020
cf0db98
address comment
LantaoJin Jun 15, 2020
479901d
fix ut
LantaoJin Jun 16, 2020
5aa4c1a
no need to rename simpleString to void
LantaoJin Jun 30, 2020
fdf57bf
check all cases in UT
LantaoJin Jul 1, 2020
de08967
address comments
LantaoJin Jul 1, 2020
d6f1a4b
replace table should check assertion
LantaoJin Jul 1, 2020
98d12fe
fix ut
LantaoJin Jul 2, 2020
31ba0bf
use unknown
LantaoJin Jul 6, 2020
9ad57d1
test case adjustment
LantaoJin Jul 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveCatalogs.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
override def apply(plan: LogicalPlan): LogicalPlan = plan resolveOperators {
case AlterTableAddColumnsStatement(
nameParts @ NonSessionCatalogAndTable(catalog, tbl), cols) =>
cols.foreach(c => failNullType(c.dataType))
cols.foreach(c => failCharType(c.dataType))
val changes = cols.map { col =>
TableChange.addColumn(
Expand All @@ -47,6 +48,7 @@ class ResolveCatalogs(val catalogManager: CatalogManager)

case AlterTableReplaceColumnsStatement(
nameParts @ NonSessionCatalogAndTable(catalog, tbl), cols) =>
cols.foreach(c => failNullType(c.dataType))
cols.foreach(c => failCharType(c.dataType))
val changes: Seq[TableChange] = loadTable(catalog, tbl.asIdentifier) match {
case Some(table) =>
Expand All @@ -69,6 +71,7 @@ class ResolveCatalogs(val catalogManager: CatalogManager)

case a @ AlterTableAlterColumnStatement(
nameParts @ NonSessionCatalogAndTable(catalog, tbl), _, _, _, _, _) =>
a.dataType.foreach(failNullType)
a.dataType.foreach(failCharType)
val colName = a.column.toArray
val typeChange = a.dataType.map { newDataType =>
Expand Down Expand Up @@ -145,6 +148,7 @@ class ResolveCatalogs(val catalogManager: CatalogManager)

case c @ CreateTableStatement(
NonSessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _) =>
assertNoNullTypeInSchema(c.tableSchema)
assertNoCharTypeInSchema(c.tableSchema)
CreateV2Table(
catalog.asTableCatalog,
Expand All @@ -157,6 +161,9 @@ class ResolveCatalogs(val catalogManager: CatalogManager)

case c @ CreateTableAsSelectStatement(
NonSessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _, _) =>
if (c.asSelect.resolved) {
assertNoNullTypeInSchema(c.asSelect.schema)
}
CreateTableAsSelect(
catalog.asTableCatalog,
tbl.asIdentifier,
Expand All @@ -172,6 +179,7 @@ class ResolveCatalogs(val catalogManager: CatalogManager)

case c @ ReplaceTableStatement(
NonSessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _) =>
assertNoNullTypeInSchema(c.tableSchema)
assertNoCharTypeInSchema(c.tableSchema)
ReplaceTable(
catalog.asTableCatalog,
Expand All @@ -184,6 +192,9 @@ class ResolveCatalogs(val catalogManager: CatalogManager)

case c @ ReplaceTableAsSelectStatement(
NonSessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _, _) =>
if (c.asSelect.resolved) {
assertNoNullTypeInSchema(c.asSelect.schema)
}
ReplaceTableAsSelect(
catalog.asTableCatalog,
tbl.asIdentifier,
Expand Down
1 change: 1 addition & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2211,6 +2211,7 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
DecimalType(precision.getText.toInt, 0)
case ("decimal" | "dec" | "numeric", precision :: scale :: Nil) =>
DecimalType(precision.getText.toInt, scale.getText.toInt)
case ("void", Nil) => NullType
Comment thread
LantaoJin marked this conversation as resolved.
case ("interval", Nil) => CalendarIntervalType
case (dt, params) =>
val dtStr = if (params.nonEmpty) s"$dt(${params.mkString(",")})" else dt
Expand Down
21 changes: 20 additions & 1 deletion sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import org.apache.spark.sql.catalyst.parser.CatalystSqlParser
import org.apache.spark.sql.catalyst.plans.logical.AlterTable
import org.apache.spark.sql.connector.catalog.TableChange._
import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Relation
import org.apache.spark.sql.types.{ArrayType, DataType, HIVE_TYPE_STRING, HiveStringType, MapType, StructField, StructType}
import org.apache.spark.sql.types.{ArrayType, DataType, HIVE_TYPE_STRING, HiveStringType, MapType, NullType, StructField, StructType}
import org.apache.spark.sql.util.CaseInsensitiveStringMap
import org.apache.spark.util.Utils

Expand Down Expand Up @@ -346,4 +346,23 @@ private[sql] object CatalogV2Util {
}
}
}

def failNullType(dt: DataType): Unit = {
def containsNullType(dt: DataType): Boolean = dt match {
case ArrayType(et, _) => containsNullType(et)
case MapType(kt, vt, _) => containsNullType(kt) || containsNullType(vt)
case StructType(fields) => fields.exists(f => containsNullType(f.dataType))
case _ => dt.isInstanceOf[NullType]
}
if (containsNullType(dt)) {
throw new AnalysisException(
"Cannot create tables with VOID type.")

@dongjoon-hyun dongjoon-hyun Jul 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ur, are we going to expose VOID here? Maybe, NULL is better at this error message? Technically, this function is preventing NullType, not Void type in Apache Spark Catalyst module.

@LantaoJin LantaoJin Jul 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloud-fan ask me to use VOID from NULL. Personally I agree to use VOID even through NullType is a data type class in Spark, but the symbol "null" has a literal syntax. Every time when we see a "null", we need to finger out it's a data type or literal. It's a little ambiguity. So @cloud-fan also suggests me to change the simpleString of NullType to "void". So make the symbol "null" only be literal.

@LantaoJin LantaoJin Jul 1, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I am not sure the behaviour in other database managements. Maybe NULL is a data type and null is a literal?

@LantaoJin LantaoJin Jul 1, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check some database system to see their behaviours, wait a minute.

@dongjoon-hyun dongjoon-hyun Jul 1, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for checking, @LantaoJin . This is also confusing to me. From Hive side, VOID is a type name. But, Apache Spark side,

@maropu maropu Jul 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unknown looks okay to me. Since null is mainly used to represent a literal in spark, I think its better to avoid using it for data types. Also, I think void is a word that is rarely used in relational databases.

@viirya viirya Jul 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically, looks like current change allows to parse void as NullType, but if it is used in a command like creating table, we throw exception?

One question, if we don't allow void/unknown type, is there good reason to accept in parser?

Because we don't have real void data type, to say "unknown" type in error message looks okay.

@LantaoJin LantaoJin Jul 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We throw exception in command like creating table.

One question, if we don't allow void/unknown type, is there good reason to accept in parser?

VOID is a legacy Hive table type which may be read by Spark. So in #28935, I map case ("void", Nil) => HiveVoidType in Parser, but https://github.com/apache/spark/pull/28935/files#r446771337 suggested me to reuse NullType. So the syntax seems we accepted void/unknown in parser. Actually, first, we accept the legacy Hive type VOID in parser and two, we fobid creating table with void/null.

@LantaoJin LantaoJin Jul 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like unknown is accepted here. I am going to patch it. Any more comments @dongjoon-hyun @cloud-fan @gatorsmile

@cloud-fan cloud-fan Jul 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with unknown then

}
}

def assertNoNullTypeInSchema(schema: StructType): Unit = {
schema.foreach { f =>
failNullType(f.dataType)
}
}
}
1 change: 1 addition & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DataTypeParserSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class DataTypeParserSuite extends SparkFunSuite {
checkDataType("varchAr(20)", StringType)
checkDataType("cHaR(27)", StringType)
checkDataType("BINARY", BinaryType)
checkDataType("void", NullType)
Comment thread
LantaoJin marked this conversation as resolved.
checkDataType("interval", CalendarIntervalType)

checkDataType("array<doublE>", ArrayType(DoubleType, true))
Expand Down
11 changes: 11 additions & 0 deletions sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class ResolveSessionCatalog(
override def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
case AlterTableAddColumnsStatement(
nameParts @ SessionCatalogAndTable(catalog, tbl), cols) =>
cols.foreach(c => failNullType(c.dataType))
loadTable(catalog, tbl.asIdentifier).collect {
case v1Table: V1Table =>
if (!DDLUtils.isHiveTable(v1Table.v1Table)) {
Expand Down Expand Up @@ -76,6 +77,7 @@ class ResolveSessionCatalog(

case AlterTableReplaceColumnsStatement(
nameParts @ SessionCatalogAndTable(catalog, tbl), cols) =>
cols.foreach(c => failNullType(c.dataType))
val changes: Seq[TableChange] = loadTable(catalog, tbl.asIdentifier) match {
case Some(_: V1Table) =>
throw new AnalysisException("REPLACE COLUMNS is only supported with v2 tables.")
Expand All @@ -100,6 +102,7 @@ class ResolveSessionCatalog(

case a @ AlterTableAlterColumnStatement(
nameParts @ SessionCatalogAndTable(catalog, tbl), _, _, _, _, _) =>
a.dataType.foreach(failNullType)
loadTable(catalog, tbl.asIdentifier).collect {
case v1Table: V1Table =>
if (!DDLUtils.isHiveTable(v1Table.v1Table)) {
Expand Down Expand Up @@ -268,6 +271,7 @@ class ResolveSessionCatalog(
// session catalog and the table provider is not v2.
case c @ CreateTableStatement(
SessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _) =>
assertNoNullTypeInSchema(c.tableSchema)

@zsxwing zsxwing Apr 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if we could add a legacy flag for such behavior change in future. This changes the behavior for both v1 and v2 Catalogs in order to fix a compatibility issue with Hive Metastore. But Hive Metastore is not the only Catalog Spark supports since we have opened the Catalog APIs in DSv2.

@cloud-fan cloud-fan Apr 14, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know any database that supports creating tables with null/void type column, so this change is not for hive compatibility but for reasonable SQL semantic.

I agree this is a breaking change that should be at least put in the migration guide. A legacy config can also be added but I can't find a reasonable use case for a null type column.

@bart-samwel bart-samwel Apr 14, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know any database that supports creating tables with null/void type column, so this change is not for hive compatibility but for reasonable SQL semantic.

I agree this is a breaking change that should be at least put in the migration guide. A legacy config can also be added but I can't find a reasonable use case for a null type column.

I think the main reason why you would want to support it is when people are using tables / views / temp tables to structure existing workloads. We support NullType type in CTEs, but in the case where people want to reuse the same CTE in multiple queries (i.e., multi-output workloads), they have no choice but to use views or temporary tables. (With DataFrames they'd still be able to reuse the same dataframe for multiple outputs, but in SQL that doesn't work.)

One typical use case where you use CTEs to structure your code is if you have multiple sources with different structures that you then UNION ALL together into a single dataset. It is not uncommon for each of the sources to have certain columns that don't apply, and then you write explicit NULLs there. It would be pretty annoying if you had to write explicit casts of those NULLs to the right type in all of those cases.

@cloud-fan cloud-fan Apr 14, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bart-samwel this makes sense, shall we also support CREATE TABLE t(c VOID)? Your case seems like CTAS only.

@bart-samwel bart-samwel Apr 14, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bart-samwel this makes sense, shall we also support CREATE TABLE t(c VOID)? Your case seems like CTAS only.

I think the CREATE TABLE case with explicit types is not very useful, but it could be useful if there were tools that get a table's schema and then try to recreate it, e.g. for mocking purposes. Probably best to be orthogonal here.

@cloud-fan cloud-fan Apr 14, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LantaoJin do you have time to fix it? I think we can simply remove the null type check and add a few tests with both in-memory and hive catalog.

val provider = c.provider.getOrElse(conf.defaultDataSourceName)
if (!isV2Provider(provider)) {
if (!DDLUtils.isHiveTable(Some(provider))) {
Expand All @@ -292,6 +296,9 @@ class ResolveSessionCatalog(

case c @ CreateTableAsSelectStatement(
SessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _, _) =>
if (c.asSelect.resolved) {
assertNoNullTypeInSchema(c.asSelect.schema)
}
val provider = c.provider.getOrElse(conf.defaultDataSourceName)
if (!isV2Provider(provider)) {
val tableDesc = buildCatalogTable(tbl.asTableIdentifier, new StructType,
Expand Down Expand Up @@ -319,6 +326,7 @@ class ResolveSessionCatalog(
// session catalog and the table provider is not v2.
case c @ ReplaceTableStatement(
SessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _) =>
assertNoNullTypeInSchema(c.tableSchema)
Comment thread
LantaoJin marked this conversation as resolved.
val provider = c.provider.getOrElse(conf.defaultDataSourceName)
if (!isV2Provider(provider)) {
throw new AnalysisException("REPLACE TABLE is only supported with v2 tables.")
Expand All @@ -336,6 +344,9 @@ class ResolveSessionCatalog(

case c @ ReplaceTableAsSelectStatement(
SessionCatalogAndTable(catalog, tbl), _, _, _, _, _, _, _, _, _, _) =>
if (c.asSelect.resolved) {
assertNoNullTypeInSchema(c.asSelect.schema)
}
val provider = c.provider.getOrElse(conf.defaultDataSourceName)
if (!isV2Provider(provider)) {
throw new AnalysisException("REPLACE TABLE AS SELECT is only supported with v2 tables.")
Expand Down
3 changes: 3 additions & 0 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import org.apache.spark.sql.catalyst.catalog._
import org.apache.spark.sql.catalyst.expressions.{Expression, InputFileBlockLength, InputFileBlockStart, InputFileName, RowOrdering}
import org.apache.spark.sql.catalyst.plans.logical._
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.connector.catalog.CatalogV2Util.assertNoNullTypeInSchema
import org.apache.spark.sql.connector.expressions.{FieldReference, RewritableTransform}
import org.apache.spark.sql.execution.command.DDLUtils
import org.apache.spark.sql.execution.datasources.v2.FileDataSourceV2
Expand Down Expand Up @@ -292,6 +293,8 @@ case class PreprocessTableCreation(sparkSession: SparkSession) extends Rule[Logi
"in the table definition of " + table.identifier,
sparkSession.sessionState.conf.caseSensitiveAnalysis)

assertNoNullTypeInSchema(schema)

@cloud-fan cloud-fan Jul 1, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? I think the changes in ResolveCatalogs and ResolveSessionCatalog should cover all the commands.

@LantaoJin LantaoJin Jul 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, "CREATE TABLE t1 USING PARQUET AS SELECT null as null_col" in Spark will throw Parquet data source does not support null data type. instead of Cannot create tables with VOID type

Comparing the error message from Hive SemanticException [Error 10305]: CREATE-TABLE-AS-SELECT creates a VOID type, please use CAST to specify the type, near field: col, it's confused. So better to keep it.

@LantaoJin LantaoJin Jul 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloud-fan

Without this, "CREATE TABLE t1 USING PARQUET AS SELECT null as null_col" in Spark will throw Parquet data source does not support null data type. instead of Cannot create tables with VOID type

Sorry, above description is incorrect. Without this, CTAS for Hive table CREATE TABLE t2 AS SELECT null as null_col will pass. No exception throws.

Seems Hive table (non-parquet/orc format) doesn't go through ResolveSessionCatalog


val normalizedPartCols = normalizePartitionColumns(schema, table)
val normalizedBucketSpec = normalizeBucketSpec(schema, table)

Expand Down
3 changes: 3 additions & 0 deletions sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveStrategies.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.apache.spark.sql.catalyst.expressions._
import org.apache.spark.sql.catalyst.planning._
import org.apache.spark.sql.catalyst.plans.logical.{InsertIntoDir, InsertIntoStatement, LogicalPlan, ScriptTransformation, Statistics}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.connector.catalog.CatalogV2Util.assertNoNullTypeInSchema
import org.apache.spark.sql.execution._
import org.apache.spark.sql.execution.command.{CreateTableCommand, DDLUtils}
import org.apache.spark.sql.execution.datasources.CreateTable
Expand Down Expand Up @@ -225,6 +226,8 @@ case class RelationConversions(
isConvertible(tableDesc) && SQLConf.get.getConf(HiveUtils.CONVERT_METASTORE_CTAS) =>
// validation is required to be done here before relation conversion.
DDLUtils.checkDataColNames(tableDesc.copy(schema = query.schema))
// This is for CREATE TABLE .. STORED AS PARQUET/ORC AS SELECT null
assertNoNullTypeInSchema(query.schema)
OptimizedCreateHiveTableAsSelectCommand(
tableDesc, query, query.output.map(_.name), mode)
}
Expand Down
121 changes: 121 additions & 0 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.analysis.{NoSuchPartitionException, TableAlreadyExistsException}
import org.apache.spark.sql.catalyst.catalog._
import org.apache.spark.sql.catalyst.parser.ParseException
import org.apache.spark.sql.connector.FakeV2Provider
import org.apache.spark.sql.connector.catalog.CatalogManager
import org.apache.spark.sql.connector.catalog.SupportsNamespaces.PROP_OWNER
import org.apache.spark.sql.execution.command.{DDLSuite, DDLUtils}
Expand Down Expand Up @@ -2309,6 +2310,126 @@ class HiveDDLSuite
}
}

test("SPARK-20680: Spark-sql do not support for void column datatype") {

@dongjoon-hyun dongjoon-hyun Jul 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you adjust this test case according to the last commit? Specifically, for the following?

  • void column datatype -> unknown column datatype
  • tabVoidType -> tabUnknownType
  • Forbid CTAS with null type -> Forbid CTAS with unknown type
  • Forbid Replace table AS SELECT with null type -> Forbid Replace table AS SELECT with unknown type

withTable("t") {
withView("tabVoidType") {
hiveClient.runSqlHive("CREATE TABLE t (t1 int)")
hiveClient.runSqlHive("INSERT INTO t VALUES (3)")
hiveClient.runSqlHive("CREATE VIEW tabVoidType AS SELECT NULL AS col FROM t")
checkAnswer(spark.table("tabVoidType"), Row(null))
// No exception shows
val desc = spark.sql("DESC tabVoidType").collect().toSeq
assert(desc.contains(Row("col", "null", null)))

@cloud-fan cloud-fan Jul 1, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we change NullType.toString to use void? to match the parser side.

@LantaoJin LantaoJin Jul 1, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NullType.toString retruns "NullType". What's this comment meaning?

@cloud-fan cloud-fan Jul 1, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean DataType.simpleString.

I think it looks better if DESC TABLE returns Row("col", "void", null) here.

@LantaoJin LantaoJin Jul 1, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add def simpleString = "void" in NullType will change many codes includes python. I revert this in commits/5aa4c1a. Now I think it is necessary. We should declare "null" is just a literal not a data type in Spark.

}
}

// Forbid CTAS with null type
withTable("t1", "t2", "t3") {
val e1 = intercept[AnalysisException] {
spark.sql("CREATE TABLE t1 USING PARQUET AS SELECT null as null_col")
}.getMessage
assert(e1.contains("Cannot create tables with VOID type"))

val e2 = intercept[AnalysisException] {
spark.sql("CREATE TABLE t2 AS SELECT null as null_col")
}.getMessage
assert(e2.contains("Cannot create tables with VOID type"))

val e3 = intercept[AnalysisException] {
spark.sql("CREATE TABLE t3 STORED AS PARQUET AS SELECT null as null_col")
}.getMessage
assert(e3.contains("Cannot create tables with VOID type"))
}

// Forbid Replace table AS SELECT with null type
withTable("t") {
val v2Source = classOf[FakeV2Provider].getName
val e = intercept[AnalysisException] {
spark.sql(s"CREATE OR REPLACE TABLE t USING $v2Source AS SELECT null as null_col")
}.getMessage
assert(e.contains("Cannot create tables with VOID type"))
}

// Forbid creating table with VOID type in Spark

@dongjoon-hyun dongjoon-hyun Jul 7, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this line and the following, it looks correct because we parse VOID in AstBuilder.

withTable("t1", "t2", "t3", "t4") {
val e1 = intercept[AnalysisException] {
spark.sql(s"CREATE TABLE t1 (v VOID) USING PARQUET")
}.getMessage
assert(e1.contains("Cannot create tables with VOID type"))
val e2 = intercept[AnalysisException] {
spark.sql(s"CREATE TABLE t2 (v VOID) USING hive")
}.getMessage
assert(e2.contains("Cannot create tables with VOID type"))
val e3 = intercept[AnalysisException] {
spark.sql(s"CREATE TABLE t3 (v VOID)")
}.getMessage
assert(e3.contains("Cannot create tables with VOID type"))
val e4 = intercept[AnalysisException] {
spark.sql(s"CREATE TABLE t4 (v VOID) STORED AS PARQUET")
}.getMessage
assert(e4.contains("Cannot create tables with VOID type"))
}

// Forbid Replace table with VOID type
withTable("t") {
val v2Source = classOf[FakeV2Provider].getName
val e = intercept[AnalysisException] {
spark.sql(s"CREATE OR REPLACE TABLE t (v VOID) USING $v2Source")
}.getMessage
assert(e.contains("Cannot create tables with VOID type"))
}

// Make sure spark.catalog.createTable with null type will fail
val schema1 = new StructType().add("c", NullType)
assertHiveTableNullType(schema1)
assertDSTableNullType(schema1)

val schema2 = new StructType()
.add("c", StructType(Seq(StructField.apply("c1", NullType))))
assertHiveTableNullType(schema2)
assertDSTableNullType(schema2)

val schema3 = new StructType().add("c", ArrayType(NullType))
assertHiveTableNullType(schema3)
assertDSTableNullType(schema3)

val schema4 = new StructType()
.add("c", MapType(StringType, NullType))
assertHiveTableNullType(schema4)
assertDSTableNullType(schema4)

val schema5 = new StructType()
.add("c", MapType(NullType, StringType))
assertHiveTableNullType(schema5)
assertDSTableNullType(schema5)
}

private def assertHiveTableNullType(schema: StructType): Unit = {
withTable("t") {
val e = intercept[AnalysisException] {
spark.catalog.createTable(
tableName = "t",
source = "hive",
schema = schema,
options = Map("fileFormat" -> "parquet"))
}.getMessage
assert(e.contains("Cannot create tables with VOID type"))
}
}

private def assertDSTableNullType(schema: StructType): Unit = {
withTable("t") {
val e = intercept[AnalysisException] {
spark.catalog.createTable(
tableName = "t",
source = "json",
schema = schema,
options = Map.empty[String, String])
}.getMessage
assert(e.contains("Cannot create tables with VOID type"))
}
}

test("SPARK-21216: join with a streaming DataFrame") {
import org.apache.spark.sql.execution.streaming.MemoryStream
import testImplicits._
Expand Down