-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-29379][SQL]SHOW FUNCTIONS show '!=', '<>' , 'between', 'case' #26053
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
911007a
ba22b62
074ee0e
9b8d63e
b066088
3d6c85d
85556a6
22b3487
60cd8a8
522193c
a285290
9f68ead
bc04f99
7ac4d16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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} | ||
|
|
||
|
|
||
|
|
@@ -170,6 +171,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'") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto, we should make sure the behavior is consistent with other operators.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
OK, here it will through exception of function not found |
||
| } | ||
|
|
||
| if (isTemp) { | ||
| if (databaseName.isDefined) { | ||
| throw new AnalysisException(s"Specifying a database in DROP TEMPORARY FUNCTION " + | ||
|
|
@@ -222,6 +228,22 @@ case class ShowFunctionsCommand( | |
| case (f, "USER") if showUserFunctions => f.unquotedString | ||
| case (f, "SYSTEM") if showSystemFunctions => f.unquotedString | ||
| } | ||
| functionNames.sorted.map(Row(_)) | ||
| // 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(FunctionsCommand.virtualOperators, pattern.getOrElse("*"))) | ||
| .sorted.map(Row(_)) | ||
| } else { | ||
| functionNames.sorted.map(Row(_)) | ||
| } | ||
|
|
||
| } | ||
| } | ||
|
|
||
| object FunctionsCommand { | ||
| // operators that do not have corresponding functions. | ||
| // They should be handled `DescribeFunctionCommand`, | ||
| // `DropFunctionCommand` and `ShowFunctionsCommand` | ||
| val virtualOperators = Seq("!=", "<>", "between", "case") | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -59,7 +60,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) | ||
| ++ FunctionsCommand.virtualOperators, pattern) | ||
| .map(Row(_)) | ||
| } | ||
|
|
||
|
|
@@ -111,6 +113,19 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession { | |
| checkKeywordsExist(sql("describe functioN abcadf"), "Function: abcadf not found.") | ||
| } | ||
|
|
||
| test("drop virtual functions") { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since the related changes are reverted, we should remove the test as well |
||
| 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)) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2064,7 +2064,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { | |
|
|
||
| test("show functions") { | ||
| withUserDefinedFunction("add_one" -> true) { | ||
| val numFunctions = FunctionRegistry.functionSet.size.toLong | ||
| val numFunctions = FunctionRegistry.functionSet.size.toLong + 4L | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah missed this one. We should use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good ideal, make it more clear. Done |
||
| assert(sql("show functions").count() === numFunctions) | ||
| assert(sql("show system functions").count() === numFunctions) | ||
| assert(sql("show all functions").count() === numFunctions) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -563,7 +563,7 @@ 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(sql("show functions").count() == numFunc + 5) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Could you help to trigger retest ? |
||
| assert(spark.catalog.listFunctions().count() == numFunc + 1) | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried
drop function '='? Does Spark fail with "function not found" or "Cannot drop native function"?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function not found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK this can be improved later. Let's leave it for now. Thanks for the investigation!