From e1a5ccfdfc4ea4355207cd121a372ad605a0fc3d Mon Sep 17 00:00:00 2001 From: Xiayun Sun Date: Wed, 21 Feb 2018 16:31:11 +0700 Subject: [PATCH 1/6] output current fields in missing fields error message --- .../apache/spark/sql/types/StructType.scala | 11 +++-- .../apache/spark/sql/StructTypeSuite.scala | 43 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala index d5011c3cb87e9..236990bcd0545 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala @@ -271,7 +271,9 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru */ def apply(name: String): StructField = { nameToField.getOrElse(name, - throw new IllegalArgumentException(s"""Field "$name" does not exist.""")) + throw new IllegalArgumentException( + s"""Field "$name" does not exist. + |Available fields: ${fieldNamesSet.mkString(",")}""".stripMargin)) } /** @@ -284,7 +286,8 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru val nonExistFields = names -- fieldNamesSet if (nonExistFields.nonEmpty) { throw new IllegalArgumentException( - s"Field ${nonExistFields.mkString(",")} does not exist.") + s"""Fields ${nonExistFields.mkString(",")} does not exist. + |Available fields: ${fieldNamesSet.mkString(",")}""".stripMargin) } // Preserve the original order of fields. StructType(fields.filter(f => names.contains(f.name))) @@ -297,7 +300,9 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru */ def fieldIndex(name: String): Int = { nameToIndex.getOrElse(name, - throw new IllegalArgumentException(s"""Field "$name" does not exist.""")) + throw new IllegalArgumentException( + s"""Field "$name" does not exist. + |Available fields: ${fieldNamesSet.mkString(",")}""".stripMargin)) } private[sql] def getFieldIndex(name: String): Option[Int] = { diff --git a/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala new file mode 100644 index 0000000000000..1736fd2a7f0e3 --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql + +import org.apache.spark.SparkFunSuite +import org.apache.spark.sql.types.StructType + +class StructTypeSuite extends SparkFunSuite{ + + test("SPARK-23462 lookup a single missing field should output existing fields") { + val s = StructType.fromDDL("a INT") + val e = intercept[IllegalArgumentException](s("b")).getMessage + assert(e.contains("Available fields: a")) + } + + test("SPARK-23462 lookup a set of missing fields should output existing fields") { + val s = StructType.fromDDL("a INT") + val e = intercept[IllegalArgumentException](s(Set("a", "b"))).getMessage + assert(e.contains("Available fields: a")) + } + + test("SPARK-23462 lookup fieldIndex for missing field should output existing fields") { + val s = StructType.fromDDL("a INT") + val e = intercept[IllegalArgumentException](s.fieldIndex("b")).getMessage + assert(e.contains("Available fields: a")) + } + +} From 22f89f8537948e68af52acc54d0ea1d979b40753 Mon Sep 17 00:00:00 2001 From: Xiayun Sun Date: Thu, 22 Feb 2018 11:10:52 +0700 Subject: [PATCH 2/6] formatting; test with multi-column StructType --- .../apache/spark/sql/StructTypeSuite.scala | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala index 1736fd2a7f0e3..49167685681be 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala @@ -20,24 +20,24 @@ package org.apache.spark.sql import org.apache.spark.SparkFunSuite import org.apache.spark.sql.types.StructType -class StructTypeSuite extends SparkFunSuite{ +class StructTypeSuite extends SparkFunSuite { test("SPARK-23462 lookup a single missing field should output existing fields") { - val s = StructType.fromDDL("a INT") - val e = intercept[IllegalArgumentException](s("b")).getMessage - assert(e.contains("Available fields: a")) + val s = StructType.fromDDL("a INT, b STRING") + val e = intercept[IllegalArgumentException](s("c")).getMessage + assert(e.contains("Available fields: a,b")) } test("SPARK-23462 lookup a set of missing fields should output existing fields") { - val s = StructType.fromDDL("a INT") - val e = intercept[IllegalArgumentException](s(Set("a", "b"))).getMessage - assert(e.contains("Available fields: a")) + val s = StructType.fromDDL("a INT, b STRING") + val e = intercept[IllegalArgumentException](s(Set("a", "c"))).getMessage + assert(e.contains("Available fields: a,b")) } test("SPARK-23462 lookup fieldIndex for missing field should output existing fields") { - val s = StructType.fromDDL("a INT") - val e = intercept[IllegalArgumentException](s.fieldIndex("b")).getMessage - assert(e.contains("Available fields: a")) + val s = StructType.fromDDL("a INT, b STRING") + val e = intercept[IllegalArgumentException](s.fieldIndex("c")).getMessage + assert(e.contains("Available fields: a,b")) } } From 0842eebbf04d6d616dc99c03d6aee5fbdaf3e7c2 Mon Sep 17 00:00:00 2001 From: Xiayun Sun Date: Wed, 28 Feb 2018 13:59:52 +0700 Subject: [PATCH 3/6] code review fix --- .../apache/spark/sql/types/StructType.scala | 8 ++++---- .../org/apache/spark/sql/StructTypeSuite.scala | 18 ++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala index 236990bcd0545..0f71328bf4cf4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala @@ -273,7 +273,7 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru nameToField.getOrElse(name, throw new IllegalArgumentException( s"""Field "$name" does not exist. - |Available fields: ${fieldNamesSet.mkString(",")}""".stripMargin)) + |Available fields: ${fieldNamesSet.mkString(", ")}""".stripMargin)) } /** @@ -286,8 +286,8 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru val nonExistFields = names -- fieldNamesSet if (nonExistFields.nonEmpty) { throw new IllegalArgumentException( - s"""Fields ${nonExistFields.mkString(",")} does not exist. - |Available fields: ${fieldNamesSet.mkString(",")}""".stripMargin) + s"""Fields ${nonExistFields.mkString(", ")} does not exist. + |Available fields: ${fieldNamesSet.mkString(", ")}""".stripMargin) } // Preserve the original order of fields. StructType(fields.filter(f => names.contains(f.name))) @@ -302,7 +302,7 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru nameToIndex.getOrElse(name, throw new IllegalArgumentException( s"""Field "$name" does not exist. - |Available fields: ${fieldNamesSet.mkString(",")}""".stripMargin)) + |Available fields: ${fieldNamesSet.mkString(", ")}""".stripMargin)) } private[sql] def getFieldIndex(name: String): Option[Int] = { diff --git a/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala index 49167685681be..d8e3cc9fec8d3 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala @@ -22,22 +22,20 @@ import org.apache.spark.sql.types.StructType class StructTypeSuite extends SparkFunSuite { - test("SPARK-23462 lookup a single missing field should output existing fields") { - val s = StructType.fromDDL("a INT, b STRING") + val s = StructType.fromDDL("a INT, b STRING") + + test("lookup a single missing field should output existing fields") { val e = intercept[IllegalArgumentException](s("c")).getMessage - assert(e.contains("Available fields: a,b")) + assert(e.contains("Available fields: a, b")) } - test("SPARK-23462 lookup a set of missing fields should output existing fields") { - val s = StructType.fromDDL("a INT, b STRING") + test("lookup a set of missing fields should output existing fields") { val e = intercept[IllegalArgumentException](s(Set("a", "c"))).getMessage - assert(e.contains("Available fields: a,b")) + assert(e.contains("Available fields: a, b")) } - test("SPARK-23462 lookup fieldIndex for missing field should output existing fields") { - val s = StructType.fromDDL("a INT, b STRING") + test("lookup fieldIndex for missing field should output existing fields") { val e = intercept[IllegalArgumentException](s.fieldIndex("c")).getMessage - assert(e.contains("Available fields: a,b")) + assert(e.contains("Available fields: a, b")) } - } From 8cdb1d52117325fcbdd1cefc9e9f0616afdb2baa Mon Sep 17 00:00:00 2001 From: Xiayun Sun Date: Wed, 28 Feb 2018 14:00:50 +0700 Subject: [PATCH 4/6] move --- .../scala/org/apache/spark/sql/types}/StructTypeSuite.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) rename sql/{core/src/test/scala/org/apache/spark/sql => catalyst/src/test/scala/org/apache/spark/sql/types}/StructTypeSuite.scala (95%) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala similarity index 95% rename from sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala rename to sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala index d8e3cc9fec8d3..c6ca8bb005429 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/StructTypeSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/StructTypeSuite.scala @@ -15,10 +15,9 @@ * limitations under the License. */ -package org.apache.spark.sql +package org.apache.spark.sql.types import org.apache.spark.SparkFunSuite -import org.apache.spark.sql.types.StructType class StructTypeSuite extends SparkFunSuite { From b2d18887d7263912c66ca7a032e1d8d4d6b6de70 Mon Sep 17 00:00:00 2001 From: Xiayun Sun Date: Mon, 5 Mar 2018 14:14:05 +0700 Subject: [PATCH 5/6] grammar --- .../src/main/scala/org/apache/spark/sql/types/StructType.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala index 0f71328bf4cf4..905e9e3bc790a 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala @@ -286,7 +286,7 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru val nonExistFields = names -- fieldNamesSet if (nonExistFields.nonEmpty) { throw new IllegalArgumentException( - s"""Fields ${nonExistFields.mkString(", ")} does not exist. + s"""Nonexistent field(s): ${nonExistFields.mkString(", ")}. |Available fields: ${fieldNamesSet.mkString(", ")}""".stripMargin) } // Preserve the original order of fields. From 78e037d06d2972062cac14fe8b503e0802a4c32e Mon Sep 17 00:00:00 2001 From: Xiayun Sun Date: Mon, 12 Mar 2018 09:14:54 +0700 Subject: [PATCH 6/6] use fieldNames to preserve order --- .../main/scala/org/apache/spark/sql/types/StructType.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala index 905e9e3bc790a..362676b252126 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/StructType.scala @@ -273,7 +273,7 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru nameToField.getOrElse(name, throw new IllegalArgumentException( s"""Field "$name" does not exist. - |Available fields: ${fieldNamesSet.mkString(", ")}""".stripMargin)) + |Available fields: ${fieldNames.mkString(", ")}""".stripMargin)) } /** @@ -287,7 +287,7 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru if (nonExistFields.nonEmpty) { throw new IllegalArgumentException( s"""Nonexistent field(s): ${nonExistFields.mkString(", ")}. - |Available fields: ${fieldNamesSet.mkString(", ")}""".stripMargin) + |Available fields: ${fieldNames.mkString(", ")}""".stripMargin) } // Preserve the original order of fields. StructType(fields.filter(f => names.contains(f.name))) @@ -302,7 +302,7 @@ case class StructType(fields: Array[StructField]) extends DataType with Seq[Stru nameToIndex.getOrElse(name, throw new IllegalArgumentException( s"""Field "$name" does not exist. - |Available fields: ${fieldNamesSet.mkString(", ")}""".stripMargin)) + |Available fields: ${fieldNames.mkString(", ")}""".stripMargin)) } private[sql] def getFieldIndex(name: String): Option[Int] = {