From ffa816e0a1cef6defb4b9f58e99af8f35b1f4802 Mon Sep 17 00:00:00 2001 From: Jose Torres Date: Wed, 28 Mar 2018 14:24:21 -0700 Subject: [PATCH 1/6] set defaults and add test --- .../spark/sql/test/TestSQLContext.scala | 2 ++ .../sql/test/TestSparkSessionSuite.scala | 28 +++++++++++++++++++ .../apache/spark/sql/hive/test/TestHive.scala | 2 ++ .../sql/hive/HiveSessionStateSuite.scala | 4 +++ 4 files changed, 36 insertions(+) create mode 100644 sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala index 4286e8a6ca2c8..3038b822beb4a 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala @@ -34,6 +34,8 @@ private[spark] class TestSparkSession(sc: SparkContext) extends SparkSession(sc) this(new SparkConf) } + SparkSession.setDefaultSession(this) + @transient override lazy val sessionState: SessionState = { new TestSQLSessionStateBuilder(this, None).build() diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala new file mode 100644 index 0000000000000..714bea45eafc8 --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala @@ -0,0 +1,28 @@ +/* + * 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.test + +import org.apache.spark.SparkFunSuite +import org.apache.spark.sql.SparkSession + +class TestSparkSessionSuite extends SparkFunSuite { + test("default session is set in constructor") { + val session = new TestSparkSession() + assert(SparkSession.getDefaultSession.contains(session)) + } +} diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala index fcf2025d34432..ed8b313dfb700 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala @@ -175,6 +175,8 @@ private[hive] class TestHiveSparkSession( loadTestTables) } + SparkSession.setDefaultSession(this) + { // set the metastore temporary configuration val metastoreTempConf = HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false) ++ Map( ConfVars.METASTORE_INTEGER_JDO_PUSHDOWN.varname -> "true", diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala index ecc09cdcdbeaf..cc7226825181c 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala @@ -49,4 +49,8 @@ class HiveSessionStateSuite extends SessionStateSuite with TestHiveSingleton { assert(oldValue == newValue, "cloneSession and then newSession should not affect the Derby directory") } + + test("default session is set") { + SparkSession.getDefaultSession.contains(activeSession) + } } From d0988f7378152b576844c4ae11b1761fa9a3bde2 Mon Sep 17 00:00:00 2001 From: Jose Torres Date: Wed, 28 Mar 2018 17:26:17 -0700 Subject: [PATCH 2/6] set in shared session and clean up --- .../org/apache/spark/sql/test/SharedSparkSession.scala | 2 ++ .../scala/org/apache/spark/sql/test/TestSQLContext.scala | 2 -- .../org/apache/spark/sql/test/TestSparkSessionSuite.scala | 7 +++---- .../scala/org/apache/spark/sql/hive/test/TestHive.scala | 2 -- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala index e758c865b908f..f2f43fee6e355 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala @@ -83,6 +83,7 @@ trait SharedSparkSession */ protected override def beforeAll(): Unit = { initializeSession() + SparkSession.setDefaultSession(_spark) // Ensure we have initialized the context before calling parent code super.beforeAll() @@ -94,6 +95,7 @@ trait SharedSparkSession protected override def afterAll(): Unit = { super.afterAll() if (_spark != null) { + SparkSession.clearDefaultSession() _spark.sessionState.catalog.reset() _spark.stop() _spark = null diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala index 3038b822beb4a..4286e8a6ca2c8 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala @@ -34,8 +34,6 @@ private[spark] class TestSparkSession(sc: SparkContext) extends SparkSession(sc) this(new SparkConf) } - SparkSession.setDefaultSession(this) - @transient override lazy val sessionState: SessionState = { new TestSQLSessionStateBuilder(this, None).build() diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala index 714bea45eafc8..e5c9933ec16d2 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala @@ -20,9 +20,8 @@ package org.apache.spark.sql.test import org.apache.spark.SparkFunSuite import org.apache.spark.sql.SparkSession -class TestSparkSessionSuite extends SparkFunSuite { - test("default session is set in constructor") { - val session = new TestSparkSession() - assert(SparkSession.getDefaultSession.contains(session)) +class TestSparkSessionSuite extends SparkFunSuite with SharedSparkSession { + test("default session is set") { + assert(SparkSession.getDefaultSession.contains(spark)) } } diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala index ed8b313dfb700..fcf2025d34432 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala @@ -175,8 +175,6 @@ private[hive] class TestHiveSparkSession( loadTestTables) } - SparkSession.setDefaultSession(this) - { // set the metastore temporary configuration val metastoreTempConf = HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false) ++ Map( ConfVars.METASTORE_INTEGER_JDO_PUSHDOWN.varname -> "true", From c6613ff8d43d30a3fb40036d37c55767e6a666d3 Mon Sep 17 00:00:00 2001 From: Jose Torres Date: Wed, 28 Mar 2018 17:29:07 -0700 Subject: [PATCH 3/6] Revert "set in shared session and clean up" This reverts commit d0988f7378152b576844c4ae11b1761fa9a3bde2. --- .../org/apache/spark/sql/test/SharedSparkSession.scala | 2 -- .../scala/org/apache/spark/sql/test/TestSQLContext.scala | 2 ++ .../org/apache/spark/sql/test/TestSparkSessionSuite.scala | 7 ++++--- .../scala/org/apache/spark/sql/hive/test/TestHive.scala | 2 ++ 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala index f2f43fee6e355..e758c865b908f 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/SharedSparkSession.scala @@ -83,7 +83,6 @@ trait SharedSparkSession */ protected override def beforeAll(): Unit = { initializeSession() - SparkSession.setDefaultSession(_spark) // Ensure we have initialized the context before calling parent code super.beforeAll() @@ -95,7 +94,6 @@ trait SharedSparkSession protected override def afterAll(): Unit = { super.afterAll() if (_spark != null) { - SparkSession.clearDefaultSession() _spark.sessionState.catalog.reset() _spark.stop() _spark = null diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala index 4286e8a6ca2c8..3038b822beb4a 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSQLContext.scala @@ -34,6 +34,8 @@ private[spark] class TestSparkSession(sc: SparkContext) extends SparkSession(sc) this(new SparkConf) } + SparkSession.setDefaultSession(this) + @transient override lazy val sessionState: SessionState = { new TestSQLSessionStateBuilder(this, None).build() diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala index e5c9933ec16d2..714bea45eafc8 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala @@ -20,8 +20,9 @@ package org.apache.spark.sql.test import org.apache.spark.SparkFunSuite import org.apache.spark.sql.SparkSession -class TestSparkSessionSuite extends SparkFunSuite with SharedSparkSession { - test("default session is set") { - assert(SparkSession.getDefaultSession.contains(spark)) +class TestSparkSessionSuite extends SparkFunSuite { + test("default session is set in constructor") { + val session = new TestSparkSession() + assert(SparkSession.getDefaultSession.contains(session)) } } diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala index fcf2025d34432..ed8b313dfb700 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala @@ -175,6 +175,8 @@ private[hive] class TestHiveSparkSession( loadTestTables) } + SparkSession.setDefaultSession(this) + { // set the metastore temporary configuration val metastoreTempConf = HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false) ++ Map( ConfVars.METASTORE_INTEGER_JDO_PUSHDOWN.varname -> "true", From 7be16a93da1efb86c69aa74f2c352ccbb66e5d4a Mon Sep 17 00:00:00 2001 From: Jose Torres Date: Wed, 28 Mar 2018 17:31:17 -0700 Subject: [PATCH 4/6] close spark session in test --- .../scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala index 714bea45eafc8..4019c6888da98 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/test/TestSparkSessionSuite.scala @@ -24,5 +24,6 @@ class TestSparkSessionSuite extends SparkFunSuite { test("default session is set in constructor") { val session = new TestSparkSession() assert(SparkSession.getDefaultSession.contains(session)) + session.stop() } } From 851a5efa87a9f10843ec9d45437d6a5d94cc0816 Mon Sep 17 00:00:00 2001 From: Jose Torres Date: Wed, 28 Mar 2018 20:18:14 -0700 Subject: [PATCH 5/6] don't double set default session in hive --- .../main/scala/org/apache/spark/sql/hive/test/TestHive.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala index ed8b313dfb700..fcf2025d34432 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala @@ -175,8 +175,6 @@ private[hive] class TestHiveSparkSession( loadTestTables) } - SparkSession.setDefaultSession(this) - { // set the metastore temporary configuration val metastoreTempConf = HiveUtils.newTemporaryConfiguration(useInMemoryDerby = false) ++ Map( ConfVars.METASTORE_INTEGER_JDO_PUSHDOWN.varname -> "true", From d9c996fce0b2b855fd635cd42b61854057f55667 Mon Sep 17 00:00:00 2001 From: Jose Torres Date: Thu, 29 Mar 2018 15:20:04 -0700 Subject: [PATCH 6/6] leave todo in hive --- .../main/scala/org/apache/spark/sql/hive/test/TestHive.scala | 4 ++++ .../org/apache/spark/sql/hive/HiveSessionStateSuite.scala | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala index fcf2025d34432..814038d4ef7af 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/test/TestHive.scala @@ -159,6 +159,10 @@ private[hive] class TestHiveSparkSession( private val loadTestTables: Boolean) extends SparkSession(sc) with Logging { self => + // TODO(SPARK-23826): TestHiveSparkSession should set default session the same way as + // TestSparkSession, but doing this the same way breaks many tests in the package. We need + // to investigate and find a different strategy. + def this(sc: SparkContext, loadTestTables: Boolean) { this( sc, diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala index cc7226825181c..ecc09cdcdbeaf 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSessionStateSuite.scala @@ -49,8 +49,4 @@ class HiveSessionStateSuite extends SessionStateSuite with TestHiveSingleton { assert(oldValue == newValue, "cloneSession and then newSession should not affect the Derby directory") } - - test("default session is set") { - SparkSession.getDefaultSession.contains(activeSession) - } }