From 08ad908728f6def2c07368a5cdd89df00f32ba2d Mon Sep 17 00:00:00 2001 From: Kaya Kupferschmidt Date: Thu, 5 Jul 2018 08:01:28 +0200 Subject: [PATCH 1/3] SPARK-24742: Fixed NullPointerException in Metadata when storing null values --- .../org/apache/spark/sql/types/Metadata.scala | 2 + .../spark/sql/types/MetadataSuite.scala | 69 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/Metadata.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/Metadata.scala index 352fb545f4b6..7c15dc0de4b6 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/types/Metadata.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/types/Metadata.scala @@ -215,6 +215,8 @@ object Metadata { x.## case x: Metadata => hash(x.map) + case null => + 0 case other => throw new RuntimeException(s"Do not support type ${other.getClass}.") } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala new file mode 100644 index 000000000000..3978c8cd6b5a --- /dev/null +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala @@ -0,0 +1,69 @@ +/* + * 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.types + +import org.apache.spark.SparkFunSuite + +class MetadataSuite extends SparkFunSuite { + test("String Metadata") { + val meta = new MetadataBuilder().putString("key", "value").build() + assert(meta.## != 0) + assert(meta.getString("key") == "value") + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getString("no_such_key")) + intercept[ClassCastException](meta.getBoolean("key")) + } + + test("Long Metadata") { + val meta = new MetadataBuilder().putLong("key", 12).build() + assert(meta.## != 0) + assert(meta.getLong("key") == 12) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getLong("no_such_key")) + intercept[ClassCastException](meta.getBoolean("key")) + } + + test("Double Metadata") { + val meta = new MetadataBuilder().putDouble("key", 12).build() + assert(meta.## != 0) + assert(meta.getDouble("key") == 12) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getDouble("no_such_key")) + intercept[ClassCastException](meta.getBoolean("key")) + } + + test("Boolean Metadata") { + val meta = new MetadataBuilder().putBoolean("key", true).build() + assert(meta.## != 0) + assert(meta.getBoolean("key") == true) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getBoolean("no_such_key")) + intercept[ClassCastException](meta.getString("key")) + } + + test("Null Metadata") { + val meta = new MetadataBuilder().putNull("key").build() + assert(meta.## != 0) + assert(meta.getString("key") == null) + assert(meta.getDouble("key") == 0) + assert(meta.getLong("key") == 0) + assert(meta.getBoolean("key") == false) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getLong("no_such_key")) + } +} From 088e2d789dad707bd657a72afa8933e957641536 Mon Sep 17 00:00:00 2001 From: Kaya Kupferschmidt Date: Thu, 5 Jul 2018 08:03:33 +0200 Subject: [PATCH 2/3] SPARK-24742: Improved TestSuite for Metadata --- .../scala/org/apache/spark/sql/types/MetadataSuite.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala index 3978c8cd6b5a..3e7a593328c1 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala @@ -22,6 +22,7 @@ import org.apache.spark.SparkFunSuite class MetadataSuite extends SparkFunSuite { test("String Metadata") { val meta = new MetadataBuilder().putString("key", "value").build() + assert(meta == meta) assert(meta.## != 0) assert(meta.getString("key") == "value") assert(meta.contains("key")) @@ -31,6 +32,7 @@ class MetadataSuite extends SparkFunSuite { test("Long Metadata") { val meta = new MetadataBuilder().putLong("key", 12).build() + assert(meta == meta) assert(meta.## != 0) assert(meta.getLong("key") == 12) assert(meta.contains("key")) @@ -40,6 +42,7 @@ class MetadataSuite extends SparkFunSuite { test("Double Metadata") { val meta = new MetadataBuilder().putDouble("key", 12).build() + assert(meta == meta) assert(meta.## != 0) assert(meta.getDouble("key") == 12) assert(meta.contains("key")) @@ -49,6 +52,7 @@ class MetadataSuite extends SparkFunSuite { test("Boolean Metadata") { val meta = new MetadataBuilder().putBoolean("key", true).build() + assert(meta == meta) assert(meta.## != 0) assert(meta.getBoolean("key") == true) assert(meta.contains("key")) @@ -58,6 +62,7 @@ class MetadataSuite extends SparkFunSuite { test("Null Metadata") { val meta = new MetadataBuilder().putNull("key").build() + assert(meta == meta) assert(meta.## != 0) assert(meta.getString("key") == null) assert(meta.getDouble("key") == 0) From 17485a703f686441ee1569b3ccfd3547af13f397 Mon Sep 17 00:00:00 2001 From: Kaya Kupferschmidt Date: Wed, 1 Aug 2018 21:34:41 +0200 Subject: [PATCH 3/3] SPARK-24742: Fixed indendation and improved assertions in unittests --- .../spark/sql/types/MetadataSuite.scala | 94 +++++++++---------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala index 3e7a593328c1..210e65708170 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/types/MetadataSuite.scala @@ -20,55 +20,55 @@ package org.apache.spark.sql.types import org.apache.spark.SparkFunSuite class MetadataSuite extends SparkFunSuite { - test("String Metadata") { - val meta = new MetadataBuilder().putString("key", "value").build() - assert(meta == meta) - assert(meta.## != 0) - assert(meta.getString("key") == "value") - assert(meta.contains("key")) - intercept[NoSuchElementException](meta.getString("no_such_key")) - intercept[ClassCastException](meta.getBoolean("key")) - } + test("String Metadata") { + val meta = new MetadataBuilder().putString("key", "value").build() + assert(meta === meta) + assert(meta.## !== 0) + assert(meta.getString("key") === "value") + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getString("no_such_key")) + intercept[ClassCastException](meta.getBoolean("key")) + } - test("Long Metadata") { - val meta = new MetadataBuilder().putLong("key", 12).build() - assert(meta == meta) - assert(meta.## != 0) - assert(meta.getLong("key") == 12) - assert(meta.contains("key")) - intercept[NoSuchElementException](meta.getLong("no_such_key")) - intercept[ClassCastException](meta.getBoolean("key")) - } + test("Long Metadata") { + val meta = new MetadataBuilder().putLong("key", 12).build() + assert(meta === meta) + assert(meta.## !== 0) + assert(meta.getLong("key") === 12) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getLong("no_such_key")) + intercept[ClassCastException](meta.getBoolean("key")) + } - test("Double Metadata") { - val meta = new MetadataBuilder().putDouble("key", 12).build() - assert(meta == meta) - assert(meta.## != 0) - assert(meta.getDouble("key") == 12) - assert(meta.contains("key")) - intercept[NoSuchElementException](meta.getDouble("no_such_key")) - intercept[ClassCastException](meta.getBoolean("key")) - } + test("Double Metadata") { + val meta = new MetadataBuilder().putDouble("key", 12).build() + assert(meta === meta) + assert(meta.## !== 0) + assert(meta.getDouble("key") === 12) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getDouble("no_such_key")) + intercept[ClassCastException](meta.getBoolean("key")) + } - test("Boolean Metadata") { - val meta = new MetadataBuilder().putBoolean("key", true).build() - assert(meta == meta) - assert(meta.## != 0) - assert(meta.getBoolean("key") == true) - assert(meta.contains("key")) - intercept[NoSuchElementException](meta.getBoolean("no_such_key")) - intercept[ClassCastException](meta.getString("key")) - } + test("Boolean Metadata") { + val meta = new MetadataBuilder().putBoolean("key", true).build() + assert(meta === meta) + assert(meta.## !== 0) + assert(meta.getBoolean("key") === true) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getBoolean("no_such_key")) + intercept[ClassCastException](meta.getString("key")) + } - test("Null Metadata") { - val meta = new MetadataBuilder().putNull("key").build() - assert(meta == meta) - assert(meta.## != 0) - assert(meta.getString("key") == null) - assert(meta.getDouble("key") == 0) - assert(meta.getLong("key") == 0) - assert(meta.getBoolean("key") == false) - assert(meta.contains("key")) - intercept[NoSuchElementException](meta.getLong("no_such_key")) - } + test("Null Metadata") { + val meta = new MetadataBuilder().putNull("key").build() + assert(meta === meta) + assert(meta.## !== 0) + assert(meta.getString("key") === null) + assert(meta.getDouble("key") === 0) + assert(meta.getLong("key") === 0) + assert(meta.getBoolean("key") === false) + assert(meta.contains("key")) + intercept[NoSuchElementException](meta.getLong("no_such_key")) + } }