From 814cddcd631c802a8ccff44c2a2cfd01ce47b4c2 Mon Sep 17 00:00:00 2001 From: Max Gekk Date: Sat, 5 Dec 2020 09:49:53 +0300 Subject: [PATCH 1/5] Add a test --- .../command/ShowPartitionsSuiteBase.scala | 28 +++++++++++++++++-- .../command/v1/ShowPartitionsSuite.scala | 4 --- .../command/v2/ShowPartitionsSuite.scala | 4 --- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala index 82457f96a3003..a9b24db3b7907 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala @@ -21,6 +21,7 @@ import org.scalactic.source.Position import org.scalatest.Tag import org.apache.spark.sql.{AnalysisException, QueryTest, Row} +import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.test.SQLTestUtils import org.apache.spark.sql.types.{StringType, StructType} @@ -28,7 +29,6 @@ trait ShowPartitionsSuiteBase extends QueryTest with SQLTestUtils { protected def version: String protected def catalog: String protected def defaultUsing: String - protected def wrongPartitionColumnsError(columns: String*): String // Gets the schema of `SHOW PARTITIONS` private val showSchema: StructType = new StructType().add("partition", StringType, false) protected def runShowPartitionsSql(sqlText: String, expected: Seq[Row]): Unit = { @@ -94,7 +94,7 @@ trait ShowPartitionsSuiteBase extends QueryTest with SQLTestUtils { val errMsg = intercept[AnalysisException] { sql(s"SHOW PARTITIONS $table PARTITION(abcd=2015, xyz=1)") }.getMessage - assert(errMsg.contains(wrongPartitionColumnsError("abcd", "xyz"))) + assert(errMsg.contains("abcd is not a valid partition column")) } } } @@ -149,4 +149,28 @@ trait ShowPartitionsSuiteBase extends QueryTest with SQLTestUtils { } } } + + test("case sensitivity of partition spec") { + withNamespace(s"$catalog.ns") { + sql(s"CREATE NAMESPACE $catalog.ns") + val t = s"$catalog.ns.part_table" + withTable(t) { + sql(s""" + |CREATE TABLE $t (price int, qty int, year int, month int) + |$defaultUsing + |partitioned by (year, month)""".stripMargin) + sql(s"INSERT INTO $t PARTITION(year = 2015, month = 1) SELECT 1, 1") + Seq( + true -> "PARTITION(year = 2015, month = 1)", + false -> "PARTITION(YEAR = 2015, Month = 1)" + ).foreach { case (caseSensitive, partitionSpec) => + withSQLConf(SQLConf.CASE_SENSITIVE.key -> caseSensitive.toString) { + runShowPartitionsSql( + s"SHOW PARTITIONS $t $partitionSpec", + Row("year=2015/month=1") :: Nil) + } + } + } + } + } } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/ShowPartitionsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/ShowPartitionsSuite.scala index 2b2bc9e63dc82..c752a5f358bb9 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/ShowPartitionsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/ShowPartitionsSuite.scala @@ -27,10 +27,6 @@ trait ShowPartitionsSuiteBase extends command.ShowPartitionsSuiteBase { override def catalog: String = CatalogManager.SESSION_CATALOG_NAME override def defaultUsing: String = "USING parquet" - override protected def wrongPartitionColumnsError(columns: String*): String = { - s"Non-partitioning column(s) ${columns.mkString("[", ", ", "]")} are specified" - } - test("show everything in the default database") { val table = "dateTable" withTable(table) { diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/ShowPartitionsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/ShowPartitionsSuite.scala index ca47a713ad604..55985a335c94b 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/ShowPartitionsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/ShowPartitionsSuite.scala @@ -32,10 +32,6 @@ class ShowPartitionsSuite extends command.ShowPartitionsSuiteBase with SharedSpa .set(s"spark.sql.catalog.$catalog", classOf[InMemoryPartitionTableCatalog].getName) .set(s"spark.sql.catalog.non_part_$catalog", classOf[InMemoryTableCatalog].getName) - override protected def wrongPartitionColumnsError(columns: String*): String = { - s"${columns.head} is not a valid partition column" - } - test("a table does not support partitioning") { val table = s"non_part_$catalog.tab1" withTable(table) { From f7696141586c9569e893f4742057d878b95f58d8 Mon Sep 17 00:00:00 2001 From: Max Gekk Date: Sat, 5 Dec 2020 09:50:24 +0300 Subject: [PATCH 2/5] Perform normalization --- .../apache/spark/sql/execution/command/tables.scala | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala index 9e3ca3c321a54..c979f05b8ac15 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala @@ -1005,13 +1005,19 @@ case class ShowPartitionsCommand( DDLUtils.verifyPartitionProviderIsHive(sparkSession, table, "SHOW PARTITIONS") + val normalizedSpec = spec.map(partitionSpec => PartitioningUtils.normalizePartitionSpec( + partitionSpec, + table.partitionColumnNames, + table.identifier.quotedString, + sparkSession.sessionState.conf.resolver)) + /** * Validate the partitioning spec by making sure all the referenced columns are * defined as partitioning columns in table definition. An AnalysisException exception is * thrown if the partitioning spec is invalid. */ - if (spec.isDefined) { - val badColumns = spec.get.keySet.filterNot(table.partitionColumnNames.contains) + if (normalizedSpec.isDefined) { + val badColumns = normalizedSpec.get.keySet.filterNot(table.partitionColumnNames.contains) if (badColumns.nonEmpty) { val badCols = badColumns.mkString("[", ", ", "]") throw new AnalysisException( @@ -1019,7 +1025,7 @@ case class ShowPartitionsCommand( } } - val partNames = catalog.listPartitionNames(tableName, spec) + val partNames = catalog.listPartitionNames(tableName, normalizedSpec) partNames.map(Row(_)) } } From b7e24818103119eb6dad9c100e91dfcd7f753ba6 Mon Sep 17 00:00:00 2001 From: Max Gekk Date: Sat, 5 Dec 2020 09:56:29 +0300 Subject: [PATCH 3/5] Remove dead code --- .../spark/sql/execution/command/tables.scala | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala index c979f05b8ac15..59adb7dd7e319 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala @@ -1005,26 +1005,18 @@ case class ShowPartitionsCommand( DDLUtils.verifyPartitionProviderIsHive(sparkSession, table, "SHOW PARTITIONS") + /** + * Normalizes the partition spec w.r.t the partition columns and case sensitivity settings, + * and validates the spec by making sure all the referenced columns are + * defined as partitioning columns in table definition. An AnalysisException exception is + * thrown if the partitioning spec is invalid. + */ val normalizedSpec = spec.map(partitionSpec => PartitioningUtils.normalizePartitionSpec( partitionSpec, table.partitionColumnNames, table.identifier.quotedString, sparkSession.sessionState.conf.resolver)) - /** - * Validate the partitioning spec by making sure all the referenced columns are - * defined as partitioning columns in table definition. An AnalysisException exception is - * thrown if the partitioning spec is invalid. - */ - if (normalizedSpec.isDefined) { - val badColumns = normalizedSpec.get.keySet.filterNot(table.partitionColumnNames.contains) - if (badColumns.nonEmpty) { - val badCols = badColumns.mkString("[", ", ", "]") - throw new AnalysisException( - s"Non-partitioning column(s) $badCols are specified for SHOW PARTITIONS") - } - } - val partNames = catalog.listPartitionNames(tableName, normalizedSpec) partNames.map(Row(_)) } From cb0ea4bbe2afb7f345b978f1e65f670878b0a2e4 Mon Sep 17 00:00:00 2001 From: Max Gekk Date: Sat, 5 Dec 2020 10:04:15 +0300 Subject: [PATCH 4/5] Minor refactoring --- .../spark/sql/execution/command/ShowPartitionsSuiteBase.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala index a9b24db3b7907..1bdbe24ca452b 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala @@ -158,7 +158,7 @@ trait ShowPartitionsSuiteBase extends QueryTest with SQLTestUtils { sql(s""" |CREATE TABLE $t (price int, qty int, year int, month int) |$defaultUsing - |partitioned by (year, month)""".stripMargin) + |PARTITIONED BY (year, month)""".stripMargin) sql(s"INSERT INTO $t PARTITION(year = 2015, month = 1) SELECT 1, 1") Seq( true -> "PARTITION(year = 2015, month = 1)", From 603823a31291491914fd4a9548ac558b6e0a9d71 Mon Sep 17 00:00:00 2001 From: Max Gekk Date: Sun, 6 Dec 2020 09:30:00 +0300 Subject: [PATCH 5/5] Add JIRA id to test's title --- .../spark/sql/execution/command/ShowPartitionsSuiteBase.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala index 1bdbe24ca452b..b695decdb3ec9 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala @@ -150,7 +150,7 @@ trait ShowPartitionsSuiteBase extends QueryTest with SQLTestUtils { } } - test("case sensitivity of partition spec") { + test("SPARK-33667: case sensitivity of partition spec") { withNamespace(s"$catalog.ns") { sql(s"CREATE NAMESPACE $catalog.ns") val t = s"$catalog.ns.part_table"