From d3cc47025df10012940f281af5db94c90fc83917 Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Tue, 4 Oct 2016 21:37:35 -0700 Subject: [PATCH 1/3] fix. --- .../sql/catalyst/catalog/interface.scala | 10 ++++-- .../spark/sql/execution/command/ddl.scala | 13 ++++++++ .../spark/sql/execution/command/tables.scala | 3 +- .../spark/sql/execution/datasources/ddl.scala | 10 +++++- .../org/apache/spark/sql/jdbc/JDBCSuite.scala | 32 +++++++++++++++++++ 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala index e52251f960ff..43f27befb6a1 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala @@ -52,9 +52,15 @@ case class CatalogStorageFormat( properties: Map[String, String]) { override def toString: String = { + val maskedProperties = properties.map { + case (password, _) if password.toLowerCase == "password" => (password, "###") + case (url, value) if url.toLowerCase == "url" && value.toLowerCase.contains("password") => + (url, "###") + case o => o + } val serdePropsToString = - if (properties.nonEmpty) { - s"Properties: " + properties.map(p => p._1 + "=" + p._2).mkString("[", ", ", "]") + if (maskedProperties.nonEmpty) { + s"Properties: " + maskedProperties.map(p => p._1 + "=" + p._2).mkString("[", ", ", "]") } else { "" } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala index 01ac89868d10..802f0adf5a59 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala @@ -703,4 +703,17 @@ object DDLUtils { } } } + + /** + * Masking credentials in the option lists. For example, in the sql plan explain output + * for JDBC data sources. + */ + def maskCredentials(options: Map[String, String]): Map[String, String] = { + options.map { + case (password, _) if password.toLowerCase == "password" => (password, "###") + case (url, value) if url.toLowerCase == "url" && value.toLowerCase.contains("password") => + (url, "###") + case o => o + } + } } 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 6a91c997bac6..e8ceb5fa47a5 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 @@ -476,7 +476,8 @@ case class DescribeTableCommand(table: TableIdentifier, isExtended: Boolean, isF describeBucketingInfo(metadata, buffer) append(buffer, "Storage Desc Parameters:", "", "") - metadata.storage.properties.foreach { case (key, value) => + val maskedProperties = DDLUtils.maskCredentials(metadata.storage.properties) + maskedProperties.foreach { case (key, value) => append(buffer, s" $key", value, "") } } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala index fa95af2648cf..7e66dccbef7c 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala @@ -22,7 +22,7 @@ import org.apache.spark.sql.catalyst.TableIdentifier import org.apache.spark.sql.catalyst.catalog.CatalogTable import org.apache.spark.sql.catalyst.plans.QueryPlan import org.apache.spark.sql.catalyst.plans.logical.{Command, LogicalPlan} -import org.apache.spark.sql.execution.command.RunnableCommand +import org.apache.spark.sql.execution.command.{DDLUtils, RunnableCommand} import org.apache.spark.sql.types._ case class CreateTable( @@ -52,6 +52,14 @@ case class CreateTempViewUsing( s"Temporary table '$tableIdent' should not have specified a database") } + override def argString: String = { + s"[tableIdent:$tableIdent " + + userSpecifiedSchema.getOrElse("") + + s"replace:$replace " + + s"provider:$provider " + + DDLUtils.maskCredentials(options) + } + def run(sparkSession: SparkSession): Seq[Row] = { val dataSource = DataSource( sparkSession, diff --git a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala index 10f15ca28068..9109c4e1394f 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala @@ -733,6 +733,38 @@ class JDBCSuite extends SparkFunSuite } } + test("hide credentials in create and describe a persistent/temp table") { + val password = "testPass" + val tableName = "tab1" + Seq("TABLE", "TEMPORARY VIEW").foreach { tableType => + withTable(tableName) { + val df = sql( + s""" + |CREATE $tableType $tableName + |USING org.apache.spark.sql.jdbc + |OPTIONS ( + | url '$urlWithUserAndPass', + | dbtable 'TEST.PEOPLE', + | user 'testUser', + | password '$password') + """.stripMargin) + + val explain = ExplainCommand(df.queryExecution.logical, extended = true) + spark.sessionState.executePlan(explain).executedPlan.executeCollect().foreach { r => + assert(!r.toString.contains(password)) + } + + sql(s"DESC FORMATTED $tableName").collect().foreach { r => + assert(!r.toString().contains(password)) + } + + sql(s"DESC EXTENDED $tableName").collect().foreach { r => + assert(!r.toString().contains(password)) + } + } + } + } + test("SPARK 12941: The data type mapping for StringType to Oracle") { val oracleDialect = JdbcDialects.get("jdbc:oracle://127.0.0.1/db") assert(oracleDialect.getJDBCType(StringType). From bc9a5082cf4576cfa7a6d4911db74bc0476c4360 Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Wed, 16 Nov 2016 22:50:36 -0800 Subject: [PATCH 2/3] address comments. --- .../catalyst/catalog/ExternalCatalogUtils.scala | 15 +++++++++++++++ .../spark/sql/catalyst/catalog/interface.scala | 14 +++----------- .../apache/spark/sql/execution/command/ddl.scala | 12 ------------ .../spark/sql/execution/command/tables.scala | 2 +- .../spark/sql/execution/datasources/ddl.scala | 6 +++--- 5 files changed, 22 insertions(+), 27 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala index b1442eec164d..817c1ab68847 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala @@ -119,3 +119,18 @@ object ExternalCatalogUtils { } } } + +object CatalogUtils { + /** + * Masking credentials in the option lists. For example, in the sql plan explain output + * for JDBC data sources. + */ + def maskCredentials(options: Map[String, String]): Map[String, String] = { + options.map { + case (key, _) if key.toLowerCase == "password" => (key, "###") + case (key, value) if key.toLowerCase == "url" && value.toLowerCase.contains("password") => + (key, "###") + case o => o + } + } +} diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala index 079af41a7e4d..c095232649e8 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala @@ -52,18 +52,10 @@ case class CatalogStorageFormat( properties: Map[String, String]) { override def toString: String = { - val maskedProperties = properties.map { - case (password, _) if password.toLowerCase == "password" => (password, "###") - case (url, value) if url.toLowerCase == "url" && value.toLowerCase.contains("password") => - (url, "###") - case o => o + val serdePropsToString = CatalogUtils.maskCredentials(properties) match { + case props if props.isEmpty => "" + case props => s"Properties: " + props.map(p => p._1 + "=" + p._2).mkString("[", ", ", "]") } - val serdePropsToString = - if (maskedProperties.nonEmpty) { - s"Properties: " + maskedProperties.map(p => p._1 + "=" + p._2).mkString("[", ", ", "]") - } else { - "" - } val output = Seq(locationUri.map("Location: " + _).getOrElse(""), inputFormat.map("InputFormat: " + _).getOrElse(""), diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala index 14d1fbe5aee3..b6d79dfc9e47 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala @@ -767,16 +767,4 @@ object DDLUtils { } } - /** - * Masking credentials in the option lists. For example, in the sql plan explain output - * for JDBC data sources. - */ - def maskCredentials(options: Map[String, String]): Map[String, String] = { - options.map { - case (password, _) if password.toLowerCase == "password" => (password, "###") - case (url, value) if url.toLowerCase == "url" && value.toLowerCase.contains("password") => - (url, "###") - case o => o - } - } } 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 0cfa17b106a3..00d67e07fbe1 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 @@ -497,7 +497,7 @@ case class DescribeTableCommand( describeBucketingInfo(metadata, buffer) append(buffer, "Storage Desc Parameters:", "", "") - val maskedProperties = DDLUtils.maskCredentials(metadata.storage.properties) + val maskedProperties = CatalogUtils.maskCredentials(metadata.storage.properties) maskedProperties.foreach { case (key, value) => append(buffer, s" $key", value, "") } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala index 92a0e223061e..94b3ad43f720 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala @@ -19,10 +19,10 @@ package org.apache.spark.sql.execution.datasources import org.apache.spark.sql._ import org.apache.spark.sql.catalyst.TableIdentifier -import org.apache.spark.sql.catalyst.catalog.CatalogTable +import org.apache.spark.sql.catalyst.catalog.{CatalogTable, CatalogUtils} import org.apache.spark.sql.catalyst.plans.QueryPlan import org.apache.spark.sql.catalyst.plans.logical.{Command, LogicalPlan} -import org.apache.spark.sql.execution.command.{DDLUtils, RunnableCommand} +import org.apache.spark.sql.execution.command.RunnableCommand import org.apache.spark.sql.types._ case class CreateTable( @@ -61,7 +61,7 @@ case class CreateTempViewUsing( userSpecifiedSchema.getOrElse("") + s"replace:$replace " + s"provider:$provider " + - DDLUtils.maskCredentials(options) + CatalogUtils.maskCredentials(options) } def run(sparkSession: SparkSession): Seq[Row] = { From 45e0ee31347752b8a5f5bbf325a536b0aae1a3e7 Mon Sep 17 00:00:00 2001 From: gatorsmile Date: Fri, 25 Nov 2016 15:47:01 -0800 Subject: [PATCH 3/3] address comments. --- .../scala/org/apache/spark/sql/catalyst/catalog/interface.scala | 2 +- .../main/scala/org/apache/spark/sql/execution/command/ddl.scala | 1 - .../scala/org/apache/spark/sql/execution/datasources/ddl.scala | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala index c095232649e8..d8bc86727e46 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala @@ -54,7 +54,7 @@ case class CatalogStorageFormat( override def toString: String = { val serdePropsToString = CatalogUtils.maskCredentials(properties) match { case props if props.isEmpty => "" - case props => s"Properties: " + props.map(p => p._1 + "=" + p._2).mkString("[", ", ", "]") + case props => "Properties: " + props.map(p => p._1 + "=" + p._2).mkString("[", ", ", "]") } val output = Seq(locationUri.map("Location: " + _).getOrElse(""), diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala index 251a28c18d78..d80b000bcc59 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala @@ -767,5 +767,4 @@ object DDLUtils { } } } - } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala index 94b3ad43f720..695ba1234d45 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/ddl.scala @@ -58,7 +58,7 @@ case class CreateTempViewUsing( override def argString: String = { s"[tableIdent:$tableIdent " + - userSpecifiedSchema.getOrElse("") + + userSpecifiedSchema.map(_ + " ").getOrElse("") + s"replace:$replace " + s"provider:$provider " + CatalogUtils.maskCredentials(options)