From 25ec746753f29acd5e248d03db48211a3876a7c1 Mon Sep 17 00:00:00 2001 From: Wenchen Fan Date: Wed, 25 Nov 2020 01:03:04 +0800 Subject: [PATCH] fix mistakes (#9) --- .../sql/catalyst/parser/AstBuilder.scala | 5 +++-- .../analysis/ResolveSessionCatalog.scala | 2 +- .../spark/sql/execution/SparkSqlParser.scala | 19 +++++++++++++++---- .../sql/hive/execution/HiveDDLSuite.scala | 7 +++---- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index bd1f17c67060b..59ea96493f33e 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -2952,8 +2952,9 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg protected def getSerdeInfo( rowFormatCtx: Seq[RowFormatContext], createFileFormatCtx: Seq[CreateFileFormatContext], - ctx: ParserRuleContext): Option[SerdeInfo] = { - validateRowFormatFileFormat(rowFormatCtx, createFileFormatCtx, ctx) + ctx: ParserRuleContext, + skipCheck: Boolean = false): Option[SerdeInfo] = { + if (!skipCheck) validateRowFormatFileFormat(rowFormatCtx, createFileFormatCtx, ctx) val rowFormatSerdeInfo = rowFormatCtx.map(visitRowFormat) val fileFormatSerdeInfo = createFileFormatCtx.map(visitCreateFileFormat) (fileFormatSerdeInfo ++ rowFormatSerdeInfo).reduceLeftOption((l, r) => l.merge(r)) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala index 3353ea655a68f..07a906e5f0d08 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala @@ -297,7 +297,7 @@ class ResolveSessionCatalog( assertNoNullTypeInSchema(c.asSelect.schema) } val (storageFormat, provider) = getStorageFormatAndProvider( - c.provider, c.options, c.location, c.serde, ctas = false) + c.provider, c.options, c.location, c.serde, ctas = true) if (!isV2Provider(provider)) { val tableDesc = buildCatalogTable(tbl.asTableIdentifier, new StructType, c.partitioning, c.bucketSpec, c.properties, provider, c.location, diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala index efe4fa35877db..5e93666ba2515 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala @@ -438,13 +438,23 @@ class SparkSqlAstBuilder extends AstBuilder { checkDuplicateClauses(ctx.TBLPROPERTIES, "TBLPROPERTIES", ctx) val provider = ctx.tableProvider.asScala.headOption.map(_.multipartIdentifier.getText) val location = visitLocationSpecList(ctx.locationSpec()) - // rowStorage used to determine CatalogStorageFormat.serde and - // CatalogStorageFormat.properties in STORED AS clause. - val serdeInfo = getSerdeInfo(ctx.rowFormat.asScala, ctx.createFileFormat.asScala, ctx) + // TODO: Do not skip serde check for CREATE TABLE LIKE. + val serdeInfo = getSerdeInfo( + ctx.rowFormat.asScala, ctx.createFileFormat.asScala, ctx, skipCheck = true) if (provider.isDefined && serdeInfo.isDefined) { operationNotAllowed(s"CREATE TABLE LIKE ... USING ... ${serdeInfo.get.describe}", ctx) } + // TODO: remove this restriction as it seems unnecessary. + serdeInfo match { + case Some(SerdeInfo(storedAs, formatClasses, serde, _)) => + if (storedAs.isEmpty && formatClasses.isEmpty && serde.isDefined) { + throw new ParseException("'ROW FORMAT' must be used with 'STORED AS'", ctx) + } + case _ => + } + + // TODO: also look at `HiveSerDe.getDefaultStorage`. val storage = toStorageFormat(location, serdeInfo, ctx) val properties = Option(ctx.tableProps).map(visitPropertyKeyValues).getOrElse(Map.empty) CreateTableLikeCommand( @@ -603,7 +613,8 @@ class SparkSqlAstBuilder extends AstBuilder { */ override def visitInsertOverwriteHiveDir( ctx: InsertOverwriteHiveDirContext): InsertDirParams = withOrigin(ctx) { - val serdeInfo = getSerdeInfo(Seq(ctx.rowFormat), Seq(ctx.createFileFormat), ctx) + val serdeInfo = getSerdeInfo( + Option(ctx.rowFormat).toSeq, Option(ctx.createFileFormat).toSeq, ctx) val path = string(ctx.path) // The path field is required if (path.isEmpty) { diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala index 0444bee143358..b8b1da4cb9db7 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala @@ -2780,7 +2780,7 @@ class HiveDDLSuite |ROW FORMAT SERDE 'org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe' """.stripMargin) }.getMessage - assert(e.contains("'ROW FORMAT' must be used with 'STORED AS'")) + assert(e.contains("Operation not allowed: CREATE TABLE LIKE ... USING ... ROW FORMAT SERDE")) // row format doesn't work with provider hive e = intercept[AnalysisException] { @@ -2791,7 +2791,7 @@ class HiveDDLSuite |WITH SERDEPROPERTIES ('test' = 'test') """.stripMargin) }.getMessage - assert(e.contains("'ROW FORMAT' must be used with 'STORED AS'")) + assert(e.contains("Operation not allowed: CREATE TABLE LIKE ... USING ... ROW FORMAT SERDE")) // row format doesn't work without 'STORED AS' e = intercept[AnalysisException] { @@ -2813,8 +2813,7 @@ class HiveDDLSuite |ROW FORMAT SERDE 'org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe' """.stripMargin) }.getMessage - assert(e.contains( - "'INPUTFORMAT hiveFormat' and 'USING provider' should not be specified both")) + assert(e.contains("Operation not allowed: CREATE TABLE LIKE ... USING ... STORED AS")) // row format works with STORED AS hive format (from hive table) spark.sql(