From 01d3e5a545eeced71b82d26a8407ea5e1d8f49ab Mon Sep 17 00:00:00 2001 From: bomeng Date: Thu, 21 Apr 2016 13:17:53 -0700 Subject: [PATCH 1/7] improve SET / SET -v command --- .../sql/execution/command/commands.scala | 23 ++++++++++++++++--- .../sql/execution/command/DDLSuite.scala | 4 ++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala index 5d00c805a6af..6f4c810ed7c3 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala @@ -32,6 +32,8 @@ import org.apache.spark.sql.execution.debug._ import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types._ +import scala.collection.immutable.ListMap + /** * A logical command that is executed for its side-effects. `RunnableCommand`s are * wrapped in `ExecutedCommand` during execution. @@ -181,7 +183,8 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm // Queries all key-value pairs that are set in the SQLConf of the sqlContext. case None => val runFunc = (sqlContext: SQLContext) => { - sqlContext.getAllConfs.map { case (k, v) => Row(k, v) }.toSeq + sqlContext.getAllConfs.toSeq.sortBy(_._1).map { case (k, v) => Row(k, v) } ++ + getEnvList(withDoc = false) } (keyValueOutput, runFunc) @@ -189,9 +192,9 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm // SQLConf of the sqlContext. case Some(("-v", None)) => val runFunc = (sqlContext: SQLContext) => { - sqlContext.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) => + sqlContext.conf.getAllDefinedConfs.sortBy(_._1).map { case (key, defaultValue, doc) => Row(key, defaultValue, doc) - } + } ++ getEnvList(withDoc = true) } val schema = StructType( StructField("key", StringType, false) :: @@ -225,6 +228,20 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm override def run(sqlContext: SQLContext): Seq[Row] = runFunc(sqlContext) + /** + * get the system environment properties as a sequence + * @param withDoc whether the result has a doc column or not + * @return the sequence of the rows containing the key/value pair of system properties + */ + private def getEnvList(withDoc: Boolean) = { + import scala.collection.JavaConverters._ + System.getenv().asScala.toSeq.sortBy(_._1).map { + case (k, v) => if (withDoc) Row(s"env:$k", v, "") else Row(s"env:$k", v) + } ++ + System.getProperties.asScala.toSeq.sortBy(_._1).map { + case (k, v) => if (withDoc) Row(s"system:$k", v, "") else Row(s"system:$k", v) + } + } } /** diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index 9ffffa0bdd6e..fbd14aa70f62 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -716,4 +716,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { } } + test("set / set -v") { + checkExistence(sql("set"), true, "env:", "system:") + checkExistence(sql("set -v"), true, "env:", "system:") + } } From 790cfda5d5d69030cdbd2231e950f259980d824b Mon Sep 17 00:00:00 2001 From: bomeng Date: Thu, 21 Apr 2016 13:27:26 -0700 Subject: [PATCH 2/7] remove unused imports --- .../org/apache/spark/sql/execution/command/commands.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala index 6f4c810ed7c3..d166ddf492c6 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala @@ -21,7 +21,7 @@ import java.util.NoSuchElementException import org.apache.spark.internal.Logging import org.apache.spark.rdd.RDD -import org.apache.spark.sql.{AnalysisException, Dataset, Row, SQLContext} +import org.apache.spark.sql.{Dataset, Row, SQLContext} import org.apache.spark.sql.catalyst.{CatalystTypeConverters, InternalRow, TableIdentifier} import org.apache.spark.sql.catalyst.errors.TreeNodeException import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference} @@ -32,8 +32,6 @@ import org.apache.spark.sql.execution.debug._ import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types._ -import scala.collection.immutable.ListMap - /** * A logical command that is executed for its side-effects. `RunnableCommand`s are * wrapped in `ExecutedCommand` during execution. From 2a077bd40ed67a3218bc803ab7be2409612b3f4d Mon Sep 17 00:00:00 2001 From: bomeng Date: Thu, 21 Apr 2016 16:13:41 -0700 Subject: [PATCH 3/7] improve the codes and fix test failure --- .../org/apache/spark/sql/execution/command/commands.scala | 5 ++--- .../apache/spark/sql/hive/execution/HiveQuerySuite.scala | 7 +++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala index d166ddf492c6..682a32622062 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/commands.scala @@ -232,11 +232,10 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm * @return the sequence of the rows containing the key/value pair of system properties */ private def getEnvList(withDoc: Boolean) = { - import scala.collection.JavaConverters._ - System.getenv().asScala.toSeq.sortBy(_._1).map { + sys.env.toSeq.sortBy(_._1).map { case (k, v) => if (withDoc) Row(s"env:$k", v, "") else Row(s"env:$k", v) } ++ - System.getProperties.asScala.toSeq.sortBy(_._1).map { + sys.props.toSeq.sortBy(_._1).map { case (k, v) => if (withDoc) Row(s"system:$k", v, "") else Row(s"system:$k", v) } } diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala index 93d63f224132..185216d36736 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala @@ -1145,12 +1145,15 @@ class HiveQuerySuite extends HiveComparisonTest with BeforeAndAfter { }.toSet conf.clear() - val expectedConfs = conf.getAllDefinedConfs.toSet + val expectedConfs = conf.getAllDefinedConfs.toSet ++ + sys.env.map { case (k, v) => (s"env:$k", v, "") }.toSet ++ + sys.props.map { case (k, v) => (s"system:$k", v, "") }.toSet assertResult(expectedConfs)(collectResults(sql("SET -v"))) // "SET" itself returns all config variables currently specified in SQLConf. // TODO: Should we be listing the default here always? probably... - assert(sql("SET").collect().size === TestHiveContext.overrideConfs.size) + val additionalSize = sys.env.size + sys.props.size + assert(sql("SET").collect().size === TestHiveContext.overrideConfs.size + additionalSize) val defaults = collectResults(sql("SET")) assertResult(Set(testKey -> testVal)) { From e3ba7167f159a119c956d46af097b6c3e4a565ec Mon Sep 17 00:00:00 2001 From: bomeng Date: Thu, 21 Apr 2016 19:45:39 -0700 Subject: [PATCH 4/7] fix the test failure --- .../scala/org/apache/spark/sql/SQLQuerySuite.scala | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala index cdd404d699a7..891159c837dd 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala @@ -1038,16 +1038,20 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext { val nonexistentKey = "nonexistent" // "set" itself returns all config variables currently specified in SQLConf. - assert(sql("SET").collect().size === TestSQLContext.overrideConfs.size) + val additionalSize = sys.env.size + sys.props.size + assert(sql("SET").collect().size === TestSQLContext.overrideConfs.size + additionalSize) + val expectedMap = TestSQLContext.overrideConfs ++ + sys.env.map { case (k, v) => (s"env:$k", v) } ++ + sys.props.map { case (k, v) => (s"system:$k", v) } sql("SET").collect().foreach { row => val key = row.getString(0) val value = row.getString(1) assert( - TestSQLContext.overrideConfs.contains(key), + expectedMap.contains(key), s"$key should exist in SQLConf.") assert( - TestSQLContext.overrideConfs(key) === value, - s"The value of $key should be ${TestSQLContext.overrideConfs(key)} instead of $value.") + expectedMap(key) === value, + s"The value of $key should be ${expectedMap(key)} instead of $value.") } val overrideConfs = sql("SET").collect() From 84243a06bcf78283d72617f7347c5e095292c3e9 Mon Sep 17 00:00:00 2001 From: bomeng Date: Thu, 21 Apr 2016 22:05:38 -0700 Subject: [PATCH 5/7] remove unnecessary test case --- .../org/apache/spark/sql/execution/command/DDLSuite.scala | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index fbd14aa70f62..15066ca18e8c 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -715,9 +715,4 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { assert(catalog.listPartitions(tableIdent).map(_.spec) == Seq(part1)) } } - - test("set / set -v") { - checkExistence(sql("set"), true, "env:", "system:") - checkExistence(sql("set -v"), true, "env:", "system:") - } } From c6805e4c80b7ccea833507a2571c4fafb5af7e49 Mon Sep 17 00:00:00 2001 From: bomeng Date: Thu, 21 Apr 2016 22:06:32 -0700 Subject: [PATCH 6/7] roll back --- .../scala/org/apache/spark/sql/execution/command/DDLSuite.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index 15066ca18e8c..a8b3ad29d46f 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -715,4 +715,5 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { assert(catalog.listPartitions(tableIdent).map(_.spec) == Seq(part1)) } } + } From 99ff8a5aae11a62a113a54e2baced73d312a6cbe Mon Sep 17 00:00:00 2001 From: bomeng Date: Thu, 21 Apr 2016 22:07:35 -0700 Subject: [PATCH 7/7] roll back --- .../scala/org/apache/spark/sql/execution/command/DDLSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index a8b3ad29d46f..9ffffa0bdd6e 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -715,5 +715,5 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { assert(catalog.listPartitions(tableIdent).map(_.spec) == Seq(part1)) } } - + }