From 911007a45f39fd3570613d73c94381eed79d3ac0 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Tue, 8 Oct 2019 14:31:24 +0800 Subject: [PATCH 01/14] fix show functions --- .../org/apache/spark/sql/execution/command/functions.scala | 2 +- .../src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala | 3 ++- .../org/apache/spark/sql/execution/command/DDLSuite.scala | 4 ++-- .../org/apache/spark/sql/hive/execution/HiveUDFSuite.scala | 4 ++-- .../org/apache/spark/sql/hive/execution/SQLQuerySuite.scala | 5 +++++ 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala index d3b2491cd705..6727d1bd65ed 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala @@ -222,6 +222,6 @@ case class ShowFunctionsCommand( case (f, "USER") if showUserFunctions => f.unquotedString case (f, "SYSTEM") if showSystemFunctions => f.unquotedString } - functionNames.sorted.map(Row(_)) + (functionNames ++ Seq("!=", "<>", "between", "case")).sorted.map(Row(_)) } } 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 1afe3976b2a1..112f9fe23121 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 @@ -80,7 +80,8 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { createFunction(functions) - checkAnswer(sql("SHOW functions"), getFunctions("*")) + checkAnswer(sql("SHOW functions"), (getFunctions("*") ++ + Seq(Row("!="), Row("<>"), Row("between"), Row("case")))) assert(sql("SHOW functions").collect().size > 200) Seq("^c*", "*e$", "log*", "*date*").foreach { pattern => 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 70b1db8e5f0d..ed185fb84d74 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 @@ -2065,12 +2065,12 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { test("show functions") { withUserDefinedFunction("add_one" -> true) { val numFunctions = FunctionRegistry.functionSet.size.toLong - assert(sql("show functions").count() === numFunctions) + assert(sql("show functions").count() === numFunctions + 4L) assert(sql("show system functions").count() === numFunctions) assert(sql("show all functions").count() === numFunctions) assert(sql("show user functions").count() === 0L) spark.udf.register("add_one", (x: Long) => x + 1) - assert(sql("show functions").count() === numFunctions + 1L) + assert(sql("show functions").count() === numFunctions + 5L) assert(sql("show system functions").count() === numFunctions) assert(sql("show all functions").count() === numFunctions + 1L) assert(sql("show user functions").count() === 1L) diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala index 587eab4a2481..ce29702edb1b 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala @@ -563,8 +563,8 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton with SQLTestUtils { checkAnswer( sql("SELECT testUDFToListInt(s) FROM inputTable"), Seq(Row(Seq(1, 2, 3)))) - assert(sql("show functions").count() == numFunc + 1) - assert(spark.catalog.listFunctions().count() == numFunc + 1) + assert(sql("show functions").count() == numFunc + 5) + assert(spark.catalog.listFunctions().count() == numFunc + 5) } } } diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala index 736a2dcfad29..308bb0a2bb75 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala @@ -192,6 +192,11 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton { allBuiltinFunctions.foreach { f => assert(allFunctions.contains(f)) } + + Seq("!=", "<>", "between", "case").foreach { f => + assert(allFunctions.contains(f)) + } + withTempDatabase { db => def createFunction(names: Seq[String]): Unit = { names.foreach { name => From ba22b627acc254a0ff9798477f6580191e1bfe4b Mon Sep 17 00:00:00 2001 From: angerszhu Date: Tue, 8 Oct 2019 16:30:22 +0800 Subject: [PATCH 02/14] fix bug --- .../org/apache/spark/sql/execution/command/functions.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala index 6727d1bd65ed..18ea71107283 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala @@ -24,6 +24,7 @@ import org.apache.spark.sql.catalyst.FunctionIdentifier import org.apache.spark.sql.catalyst.analysis.{FunctionRegistry, NoSuchFunctionException} import org.apache.spark.sql.catalyst.catalog.{CatalogFunction, FunctionResource} import org.apache.spark.sql.catalyst.expressions.{Attribute, ExpressionInfo} +import org.apache.spark.sql.catalyst.util.StringUtils import org.apache.spark.sql.types.{StringType, StructField, StructType} @@ -222,6 +223,8 @@ case class ShowFunctionsCommand( case (f, "USER") if showUserFunctions => f.unquotedString case (f, "SYSTEM") if showSystemFunctions => f.unquotedString } - (functionNames ++ Seq("!=", "<>", "between", "case")).sorted.map(Row(_)) + (functionNames ++ + StringUtils.filterPattern(Seq("!=", "<>", "between", "case"), pattern.getOrElse("*"))) + .sorted.map(Row(_)) } } From 074ee0eb89357c699d686433e46c2b426bf7e58b Mon Sep 17 00:00:00 2001 From: angerszhu Date: Tue, 8 Oct 2019 16:36:31 +0800 Subject: [PATCH 03/14] fit SYSTEM/USER --- .../spark/sql/execution/command/functions.scala | 11 ++++++++--- .../apache/spark/sql/execution/command/DDLSuite.scala | 8 ++++---- .../spark/sql/hive/execution/HiveUDFSuite.scala | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala index 18ea71107283..218b7564c687 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala @@ -223,8 +223,13 @@ case class ShowFunctionsCommand( case (f, "USER") if showUserFunctions => f.unquotedString case (f, "SYSTEM") if showSystemFunctions => f.unquotedString } - (functionNames ++ - StringUtils.filterPattern(Seq("!=", "<>", "between", "case"), pattern.getOrElse("*"))) - .sorted.map(Row(_)) + if (showSystemFunctions) { + (functionNames ++ + StringUtils.filterPattern(Seq("!=", "<>", "between", "case"), pattern.getOrElse("*"))) + .sorted.map(Row(_)) + } else { + functionNames.sorted.map(Row(_)) + } + } } 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 ed185fb84d74..c1760e2772a3 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 @@ -2066,13 +2066,13 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { withUserDefinedFunction("add_one" -> true) { val numFunctions = FunctionRegistry.functionSet.size.toLong assert(sql("show functions").count() === numFunctions + 4L) - assert(sql("show system functions").count() === numFunctions) - assert(sql("show all functions").count() === numFunctions) + assert(sql("show system functions").count() === numFunctions + 4L) + assert(sql("show all functions").count() === numFunctions + 4L) assert(sql("show user functions").count() === 0L) spark.udf.register("add_one", (x: Long) => x + 1) assert(sql("show functions").count() === numFunctions + 5L) - assert(sql("show system functions").count() === numFunctions) - assert(sql("show all functions").count() === numFunctions + 1L) + assert(sql("show system functions").count() === numFunctions + 4L) + assert(sql("show all functions").count() === numFunctions + 5L) assert(sql("show user functions").count() === 1L) } } diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala index ce29702edb1b..8aac1c7dab0c 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala @@ -564,7 +564,7 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton with SQLTestUtils { sql("SELECT testUDFToListInt(s) FROM inputTable"), Seq(Row(Seq(1, 2, 3)))) assert(sql("show functions").count() == numFunc + 5) - assert(spark.catalog.listFunctions().count() == numFunc + 5) + assert(spark.catalog.listFunctions().count() == numFunc + 1) } } } From 9b8d63e7ce76740eb0fc08fcd2b24849ff7693be Mon Sep 17 00:00:00 2001 From: angerszhu Date: Tue, 8 Oct 2019 17:16:01 +0800 Subject: [PATCH 04/14] fix UT --- .../src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 112f9fe23121..1a593923433c 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 @@ -87,7 +87,9 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { Seq("^c*", "*e$", "log*", "*date*").foreach { pattern => // For the pattern part, only '*' and '|' are allowed as wildcards. // For '*', we need to replace it to '.*'. - checkAnswer(sql(s"SHOW FUNCTIONS '$pattern'"), getFunctions(pattern)) + checkAnswer(sql(s"SHOW FUNCTIONS '$pattern'"), + getFunctions(pattern) ++ + StringUtils.filterPattern(Seq("!=", "<>", "between", "case"), pattern).map(Row(_))) } dropFunction(functions) } From b066088ee9a7deba990327f92a82940b85bf6025 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 9 Oct 2019 09:55:04 +0800 Subject: [PATCH 05/14] follow comment --- .../scala/org/apache/spark/sql/SQLQuerySuite.scala | 10 ++++------ .../spark/sql/execution/command/DDLSuite.scala | 14 +++++++------- 2 files changed, 11 insertions(+), 13 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 1a593923433c..b3445108b050 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 @@ -59,7 +59,8 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { test("show functions") { def getFunctions(pattern: String): Seq[Row] = { StringUtils.filterPattern( - spark.sessionState.catalog.listFunctions("default").map(_._1.funcName), pattern) + spark.sessionState.catalog.listFunctions("default").map(_._1.funcName) + ++ Seq("!=", "<>", "between", "case"), pattern) .map(Row(_)) } @@ -80,16 +81,13 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { createFunction(functions) - checkAnswer(sql("SHOW functions"), (getFunctions("*") ++ - Seq(Row("!="), Row("<>"), Row("between"), Row("case")))) + checkAnswer(sql("SHOW functions"), getFunctions("*")) assert(sql("SHOW functions").collect().size > 200) Seq("^c*", "*e$", "log*", "*date*").foreach { pattern => // For the pattern part, only '*' and '|' are allowed as wildcards. // For '*', we need to replace it to '.*'. - checkAnswer(sql(s"SHOW FUNCTIONS '$pattern'"), - getFunctions(pattern) ++ - StringUtils.filterPattern(Seq("!=", "<>", "between", "case"), pattern).map(Row(_))) + checkAnswer(sql(s"SHOW FUNCTIONS '$pattern'"), getFunctions(pattern)) } dropFunction(functions) } 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 c1760e2772a3..a28ca71f81c2 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 @@ -2064,15 +2064,15 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { test("show functions") { withUserDefinedFunction("add_one" -> true) { - val numFunctions = FunctionRegistry.functionSet.size.toLong - assert(sql("show functions").count() === numFunctions + 4L) - assert(sql("show system functions").count() === numFunctions + 4L) - assert(sql("show all functions").count() === numFunctions + 4L) + val numFunctions = FunctionRegistry.functionSet.size.toLong + 4L + assert(sql("show functions").count() === numFunctions) + assert(sql("show system functions").count() === numFunctions) + assert(sql("show all functions").count() === numFunctions) assert(sql("show user functions").count() === 0L) spark.udf.register("add_one", (x: Long) => x + 1) - assert(sql("show functions").count() === numFunctions + 5L) - assert(sql("show system functions").count() === numFunctions + 4L) - assert(sql("show all functions").count() === numFunctions + 5L) + assert(sql("show functions").count() === numFunctions + 1L) + assert(sql("show system functions").count() === numFunctions) + assert(sql("show all functions").count() === numFunctions + 1L) assert(sql("show user functions").count() === 1L) } } From 3d6c85d78bf1708cd97f27c98216b7ff88c5fa71 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 16 Oct 2019 14:56:27 +0800 Subject: [PATCH 06/14] follow comment --- .../sql/execution/command/functions.scala | 21 ++++++++++++++++++- .../org/apache/spark/sql/SQLQuerySuite.scala | 16 +++++++++++++- .../sql/hive/execution/SQLQuerySuite.scala | 4 ++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala index 218b7564c687..3491d0f24999 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala @@ -73,6 +73,11 @@ case class CreateFunctionCommand( s"is not allowed: '${databaseName.get}'") } + // Redefine a virtual function is not allowed + if (FunctionsCommand.virtualOperators.contains(functionName.toLowerCase(Locale.ROOT))) { + throw new AnalysisException(s"It's not allowed to redefine virtual function '$functionName'") + } + override def run(sparkSession: SparkSession): Seq[Row] = { val catalog = sparkSession.sessionState.catalog val func = CatalogFunction(FunctionIdentifier(functionName, databaseName), className, resources) @@ -171,6 +176,11 @@ case class DropFunctionCommand( override def run(sparkSession: SparkSession): Seq[Row] = { val catalog = sparkSession.sessionState.catalog + + if (FunctionsCommand.virtualOperators.contains(functionName.toLowerCase(Locale.ROOT))) { + throw new AnalysisException(s"Cannot drop virtual function '$functionName'") + } + if (isTemp) { if (databaseName.isDefined) { throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " + @@ -223,9 +233,11 @@ case class ShowFunctionsCommand( case (f, "USER") if showUserFunctions => f.unquotedString case (f, "SYSTEM") if showSystemFunctions => f.unquotedString } + // Hard code "<>", "!=", "between", and "case" for now as there is no corresponding functions. + // "<>", "!=", "between", and "case" is SystemFunctions, only show when showSystemFunctions=true if (showSystemFunctions) { (functionNames ++ - StringUtils.filterPattern(Seq("!=", "<>", "between", "case"), pattern.getOrElse("*"))) + StringUtils.filterPattern(FunctionsCommand.virtualOperators, pattern.getOrElse("*"))) .sorted.map(Row(_)) } else { functionNames.sorted.map(Row(_)) @@ -233,3 +245,10 @@ case class ShowFunctionsCommand( } } + +object FunctionsCommand { + // operators that do not have corresponding functions. + // They should be handled in `CreateFunctionCommand`, `DescribeFunctionCommand`, + // `DropFunctionCommand` and `ShowFunctionsCommand` + val virtualOperators = Seq("!=", "<>", "between", "case") +} 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 b3445108b050..5a941163a8eb 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 @@ -31,6 +31,7 @@ import org.apache.spark.sql.catalyst.util.StringUtils import org.apache.spark.sql.execution.HiveResult.hiveResultString import org.apache.spark.sql.execution.aggregate.{HashAggregateExec, SortAggregateExec} import org.apache.spark.sql.execution.columnar.InMemoryTableScanExec +import org.apache.spark.sql.execution.command.FunctionsCommand import org.apache.spark.sql.execution.datasources.v2.BatchScanExec import org.apache.spark.sql.execution.datasources.v2.orc.OrcScan import org.apache.spark.sql.execution.datasources.v2.parquet.ParquetScan @@ -60,7 +61,7 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { def getFunctions(pattern: String): Seq[Row] = { StringUtils.filterPattern( spark.sessionState.catalog.listFunctions("default").map(_._1.funcName) - ++ Seq("!=", "<>", "between", "case"), pattern) + ++ FunctionsCommand.virtualOperators, pattern) .map(Row(_)) } @@ -112,6 +113,19 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { checkKeywordsExist(sql("describe functioN abcadf"), "Function: abcadf not found.") } + test("drop virtual functions") { + val e1 = intercept[AnalysisException] { + sql( + "drop function case") + } + assert(e1.message == "Cannot drop virtual function 'case'") + val e2 = intercept[AnalysisException] { + sql( + "drop function `!=`") + } + assert(e2.message == "Cannot drop virtual function '!='") + } + test("SPARK-14415: All functions should have own descriptions") { for (f <- spark.sessionState.functionRegistry.listFunction()) { if (!Seq("cube", "grouping", "grouping_id", "rollup", "window").contains(f.unquotedString)) { diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala index 308bb0a2bb75..d2ca434bc211 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala @@ -33,7 +33,7 @@ import org.apache.spark.sql.catalyst.analysis.{EliminateSubqueryAliases, Functio import org.apache.spark.sql.catalyst.catalog.{CatalogTableType, CatalogUtils, HiveTableRelation} import org.apache.spark.sql.catalyst.parser.ParseException import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias} -import org.apache.spark.sql.execution.command.LoadDataCommand +import org.apache.spark.sql.execution.command.{FunctionsCommand, LoadDataCommand} import org.apache.spark.sql.execution.datasources.{HadoopFsRelation, LogicalRelation} import org.apache.spark.sql.functions._ import org.apache.spark.sql.hive.{HiveExternalCatalog, HiveUtils} @@ -193,7 +193,7 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton { assert(allFunctions.contains(f)) } - Seq("!=", "<>", "between", "case").foreach { f => + FunctionsCommand.virtualOperators.foreach { f => assert(allFunctions.contains(f)) } From 85556a67e6b3c145c6b0a175814a4e276b73083b Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 16 Oct 2019 17:07:42 +0800 Subject: [PATCH 07/14] follow comment --- .../org/apache/spark/sql/execution/command/functions.scala | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala index 3491d0f24999..03ec72643653 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala @@ -73,11 +73,6 @@ case class CreateFunctionCommand( s"is not allowed: '${databaseName.get}'") } - // Redefine a virtual function is not allowed - if (FunctionsCommand.virtualOperators.contains(functionName.toLowerCase(Locale.ROOT))) { - throw new AnalysisException(s"It's not allowed to redefine virtual function '$functionName'") - } - override def run(sparkSession: SparkSession): Seq[Row] = { val catalog = sparkSession.sessionState.catalog val func = CatalogFunction(FunctionIdentifier(functionName, databaseName), className, resources) @@ -248,7 +243,7 @@ case class ShowFunctionsCommand( object FunctionsCommand { // operators that do not have corresponding functions. - // They should be handled in `CreateFunctionCommand`, `DescribeFunctionCommand`, + // They should be handled `DescribeFunctionCommand`, // `DropFunctionCommand` and `ShowFunctionsCommand` val virtualOperators = Seq("!=", "<>", "between", "case") } From 22b3487668539f6469009cb79aa740b2ff7eacd7 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 16 Oct 2019 20:53:28 +0800 Subject: [PATCH 08/14] follow comment --- .../org/apache/spark/sql/execution/command/functions.scala | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala index 03ec72643653..5f904e6ff21d 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala @@ -172,10 +172,6 @@ case class DropFunctionCommand( override def run(sparkSession: SparkSession): Seq[Row] = { val catalog = sparkSession.sessionState.catalog - if (FunctionsCommand.virtualOperators.contains(functionName.toLowerCase(Locale.ROOT))) { - throw new AnalysisException(s"Cannot drop virtual function '$functionName'") - } - if (isTemp) { if (databaseName.isDefined) { throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " + @@ -243,7 +239,6 @@ case class ShowFunctionsCommand( object FunctionsCommand { // operators that do not have corresponding functions. - // They should be handled `DescribeFunctionCommand`, - // `DropFunctionCommand` and `ShowFunctionsCommand` + // They should be handled `DescribeFunctionCommand`, `ShowFunctionsCommand` val virtualOperators = Seq("!=", "<>", "between", "case") } From 60cd8a80ab66d6138fc3e1c00f326b1260cc2814 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 16 Oct 2019 23:08:44 +0800 Subject: [PATCH 09/14] Update SQLQuerySuite.scala --- .../org/apache/spark/sql/SQLQuerySuite.scala | 21 ------------------- 1 file changed, 21 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 5a941163a8eb..91b9d3348f7a 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 @@ -113,27 +113,6 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { checkKeywordsExist(sql("describe functioN abcadf"), "Function: abcadf not found.") } - test("drop virtual functions") { - val e1 = intercept[AnalysisException] { - sql( - "drop function case") - } - assert(e1.message == "Cannot drop virtual function 'case'") - val e2 = intercept[AnalysisException] { - sql( - "drop function `!=`") - } - assert(e2.message == "Cannot drop virtual function '!='") - } - - test("SPARK-14415: All functions should have own descriptions") { - for (f <- spark.sessionState.functionRegistry.listFunction()) { - if (!Seq("cube", "grouping", "grouping_id", "rollup", "window").contains(f.unquotedString)) { - checkKeywordsNotExist(sql(s"describe function `$f`"), "N/A.") - } - } - } - test("using _FUNC_ instead of function names in examples") { val exampleRe = "(>.*;)".r val setStmtRe = "(?i)^(>\\s+set\\s+).+".r From 522193cb59f420dc5c31d375229ab3fc57ed47f3 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 16 Oct 2019 23:09:54 +0800 Subject: [PATCH 10/14] Revert "Update SQLQuerySuite.scala" This reverts commit 60cd8a80ab66d6138fc3e1c00f326b1260cc2814. --- .../org/apache/spark/sql/SQLQuerySuite.scala | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) 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 91b9d3348f7a..5a941163a8eb 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 @@ -113,6 +113,27 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { checkKeywordsExist(sql("describe functioN abcadf"), "Function: abcadf not found.") } + test("drop virtual functions") { + val e1 = intercept[AnalysisException] { + sql( + "drop function case") + } + assert(e1.message == "Cannot drop virtual function 'case'") + val e2 = intercept[AnalysisException] { + sql( + "drop function `!=`") + } + assert(e2.message == "Cannot drop virtual function '!='") + } + + test("SPARK-14415: All functions should have own descriptions") { + for (f <- spark.sessionState.functionRegistry.listFunction()) { + if (!Seq("cube", "grouping", "grouping_id", "rollup", "window").contains(f.unquotedString)) { + checkKeywordsNotExist(sql(s"describe function `$f`"), "N/A.") + } + } + } + test("using _FUNC_ instead of function names in examples") { val exampleRe = "(>.*;)".r val setStmtRe = "(?i)^(>\\s+set\\s+).+".r From a285290a97747378e162f03d000c9ffd40a141b2 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 16 Oct 2019 23:10:14 +0800 Subject: [PATCH 11/14] Update SQLQuerySuite.scala --- .../scala/org/apache/spark/sql/SQLQuerySuite.scala | 13 ------------- 1 file changed, 13 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 5a941163a8eb..630489ad9c60 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 @@ -113,19 +113,6 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { checkKeywordsExist(sql("describe functioN abcadf"), "Function: abcadf not found.") } - test("drop virtual functions") { - val e1 = intercept[AnalysisException] { - sql( - "drop function case") - } - assert(e1.message == "Cannot drop virtual function 'case'") - val e2 = intercept[AnalysisException] { - sql( - "drop function `!=`") - } - assert(e2.message == "Cannot drop virtual function '!='") - } - test("SPARK-14415: All functions should have own descriptions") { for (f <- spark.sessionState.functionRegistry.listFunction()) { if (!Seq("cube", "grouping", "grouping_id", "rollup", "window").contains(f.unquotedString)) { From 9f68ead52d9e0e55c7fe58493e15c968dfda879b Mon Sep 17 00:00:00 2001 From: angerszhu Date: Wed, 16 Oct 2019 23:18:17 +0800 Subject: [PATCH 12/14] remove empty line --- .../scala/org/apache/spark/sql/execution/command/functions.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala index 5f904e6ff21d..6fdc7f4a5819 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala @@ -171,7 +171,6 @@ case class DropFunctionCommand( override def run(sparkSession: SparkSession): Seq[Row] = { val catalog = sparkSession.sessionState.catalog - if (isTemp) { if (databaseName.isDefined) { throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " + From bc04f99fcb7ac98b12acbfdb19df332595b94003 Mon Sep 17 00:00:00 2001 From: angerszhu Date: Thu, 17 Oct 2019 14:03:25 +0800 Subject: [PATCH 13/14] follow commment --- .../org/apache/spark/sql/execution/command/DDLSuite.scala | 2 +- .../org/apache/spark/sql/hive/execution/HiveUDFSuite.scala | 4 +++- 2 files changed, 4 insertions(+), 2 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 a28ca71f81c2..ef6f0359094c 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 @@ -2064,7 +2064,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { test("show functions") { withUserDefinedFunction("add_one" -> true) { - val numFunctions = FunctionRegistry.functionSet.size.toLong + 4L + val numFunctions = FunctionRegistry.functionSet.size.toLong + FunctionsCommand.virtualOperators.size assert(sql("show functions").count() === numFunctions) assert(sql("show system functions").count() === numFunctions) assert(sql("show all functions").count() === numFunctions) diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala index 8aac1c7dab0c..aa694ea274d7 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveUDFSuite.scala @@ -32,6 +32,7 @@ import org.apache.hadoop.io.{LongWritable, Writable} import org.apache.spark.sql.{AnalysisException, QueryTest, Row} import org.apache.spark.sql.catalyst.plans.logical.Project +import org.apache.spark.sql.execution.command.FunctionsCommand import org.apache.spark.sql.functions.max import org.apache.spark.sql.hive.test.TestHiveSingleton import org.apache.spark.sql.internal.SQLConf @@ -563,7 +564,8 @@ class HiveUDFSuite extends QueryTest with TestHiveSingleton with SQLTestUtils { checkAnswer( sql("SELECT testUDFToListInt(s) FROM inputTable"), Seq(Row(Seq(1, 2, 3)))) - assert(sql("show functions").count() == numFunc + 5) + assert(sql("show functions").count() == + numFunc + FunctionsCommand.virtualOperators.size + 1) assert(spark.catalog.listFunctions().count() == numFunc + 1) } } From 7ac4d1612a99c3b2aaec93accab92a5c7dfea8aa Mon Sep 17 00:00:00 2001 From: angerszhu Date: Thu, 17 Oct 2019 14:17:22 +0800 Subject: [PATCH 14/14] scalastyle --- .../org/apache/spark/sql/execution/command/DDLSuite.scala | 3 ++- 1 file changed, 2 insertions(+), 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 ef6f0359094c..6bd240431de8 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 @@ -2064,7 +2064,8 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { test("show functions") { withUserDefinedFunction("add_one" -> true) { - val numFunctions = FunctionRegistry.functionSet.size.toLong + FunctionsCommand.virtualOperators.size + val numFunctions = FunctionRegistry.functionSet.size.toLong + + FunctionsCommand.virtualOperators.size.toLong assert(sql("show functions").count() === numFunctions) assert(sql("show system functions").count() === numFunctions) assert(sql("show all functions").count() === numFunctions)