From f953ca56eccdaef29ac580d44613a028415ba3f5 Mon Sep 17 00:00:00 2001
From: Wenchen Fan <wenchen@databricks.com>
Date: Mon, 3 Jul 2017 10:51:44 -0700
Subject: [PATCH] [SPARK-21284][SQL] rename SessionCatalog.registerFunction
parameter name
## What changes were proposed in this pull request?
Looking at the code in `SessionCatalog.registerFunction`, the parameter `ignoreIfExists` is a wrong name. When `ignoreIfExists` is true, we will override the function if it already exists. So `overrideIfExists` should be the corrected name.
## How was this patch tested?
N/A
Author: Wenchen Fan <wenchen@databricks.com>
Closes #18510 from cloud-fan/minor.
---
.../sql/catalyst/catalog/SessionCatalog.scala | 6 +++---
.../catalog/SessionCatalogSuite.scala | 20 ++++++++++---------
.../sql/execution/command/functions.scala | 3 +--
.../spark/sql/internal/CatalogSuite.scala | 2 +-
.../spark/sql/hive/HiveSessionCatalog.scala | 2 +-
.../ObjectHashAggregateExecBenchmark.scala | 2 +-
6 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
index 7ece77df7f..a86604e435 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
@@ -1104,10 +1104,10 @@ class SessionCatalog(
*/
def registerFunction(
funcDefinition: CatalogFunction,
- ignoreIfExists: Boolean,
+ overrideIfExists: Boolean,
functionBuilder: Option[FunctionBuilder] = None): Unit = {
val func = funcDefinition.identifier
- if (functionRegistry.functionExists(func) && !ignoreIfExists) {
+ if (functionRegistry.functionExists(func) && !overrideIfExists) {
throw new AnalysisException(s"Function $func already exists")
}
val info = new ExpressionInfo(funcDefinition.className, func.database.orNull, func.funcName)
@@ -1219,7 +1219,7 @@ class SessionCatalog(
// catalog. So, it is possible that qualifiedName is not exactly the same as
// catalogFunction.identifier.unquotedString (difference is on case-sensitivity).
// At here, we preserve the input from the user.
- registerFunction(catalogFunction.copy(identifier = qualifiedName), ignoreIfExists = false)
+ registerFunction(catalogFunction.copy(identifier = qualifiedName), overrideIfExists = false)
// Now, we need to create the Expression.
functionRegistry.lookupFunction(qualifiedName, children)
}
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala
index fc3893e197..8f856a0daa 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala
@@ -1175,9 +1175,9 @@ abstract class SessionCatalogSuite extends AnalysisTest {
val tempFunc1 = (e: Seq[Expression]) => e.head
val tempFunc2 = (e: Seq[Expression]) => e.last
catalog.registerFunction(
- newFunc("temp1", None), ignoreIfExists = false, functionBuilder = Some(tempFunc1))
+ newFunc("temp1", None), overrideIfExists = false, functionBuilder = Some(tempFunc1))
catalog.registerFunction(
- newFunc("temp2", None), ignoreIfExists = false, functionBuilder = Some(tempFunc2))
+ newFunc("temp2", None), overrideIfExists = false, functionBuilder = Some(tempFunc2))
val arguments = Seq(Literal(1), Literal(2), Literal(3))
assert(catalog.lookupFunction(FunctionIdentifier("temp1"), arguments) === Literal(1))
assert(catalog.lookupFunction(FunctionIdentifier("temp2"), arguments) === Literal(3))
@@ -1189,12 +1189,12 @@ abstract class SessionCatalogSuite extends AnalysisTest {
// Temporary function already exists
val e = intercept[AnalysisException] {
catalog.registerFunction(
- newFunc("temp1", None), ignoreIfExists = false, functionBuilder = Some(tempFunc3))
+ newFunc("temp1", None), overrideIfExists = false, functionBuilder = Some(tempFunc3))
}.getMessage
assert(e.contains("Function temp1 already exists"))
// Temporary function is overridden
catalog.registerFunction(
- newFunc("temp1", None), ignoreIfExists = true, functionBuilder = Some(tempFunc3))
+ newFunc("temp1", None), overrideIfExists = true, functionBuilder = Some(tempFunc3))
assert(
catalog.lookupFunction(
FunctionIdentifier("temp1"), arguments) === Literal(arguments.length))
@@ -1208,7 +1208,7 @@ abstract class SessionCatalogSuite extends AnalysisTest {
val tempFunc1 = (e: Seq[Expression]) => e.head
catalog.registerFunction(
- newFunc("temp1", None), ignoreIfExists = false, functionBuilder = Some(tempFunc1))
+ newFunc("temp1", None), overrideIfExists = false, functionBuilder = Some(tempFunc1))
// Returns true when the function is temporary
assert(catalog.isTemporaryFunction(FunctionIdentifier("temp1")))
@@ -1259,7 +1259,7 @@ abstract class SessionCatalogSuite extends AnalysisTest {
withBasicCatalog { catalog =>
val tempFunc = (e: Seq[Expression]) => e.head
catalog.registerFunction(
- newFunc("func1", None), ignoreIfExists = false, functionBuilder = Some(tempFunc))
+ newFunc("func1", None), overrideIfExists = false, functionBuilder = Some(tempFunc))
val arguments = Seq(Literal(1), Literal(2), Literal(3))
assert(catalog.lookupFunction(FunctionIdentifier("func1"), arguments) === Literal(1))
catalog.dropTempFunction("func1", ignoreIfNotExists = false)
@@ -1300,7 +1300,7 @@ abstract class SessionCatalogSuite extends AnalysisTest {
withBasicCatalog { catalog =>
val tempFunc1 = (e: Seq[Expression]) => e.head
catalog.registerFunction(
- newFunc("func1", None), ignoreIfExists = false, functionBuilder = Some(tempFunc1))
+ newFunc("func1", None), overrideIfExists = false, functionBuilder = Some(tempFunc1))
assert(catalog.lookupFunction(
FunctionIdentifier("func1"), Seq(Literal(1), Literal(2), Literal(3))) == Literal(1))
catalog.dropTempFunction("func1", ignoreIfNotExists = false)
@@ -1318,8 +1318,10 @@ abstract class SessionCatalogSuite extends AnalysisTest {
val tempFunc2 = (e: Seq[Expression]) => e.last
catalog.createFunction(newFunc("func2", Some("db2")), ignoreIfExists = false)
catalog.createFunction(newFunc("not_me", Some("db2")), ignoreIfExists = false)
- catalog.registerFunction(funcMeta1, ignoreIfExists = false, functionBuilder = Some(tempFunc1))
- catalog.registerFunction(funcMeta2, ignoreIfExists = false, functionBuilder = Some(tempFunc2))
+ catalog.registerFunction(
+ funcMeta1, overrideIfExists = false, functionBuilder = Some(tempFunc1))
+ catalog.registerFunction(
+ funcMeta2, overrideIfExists = false, functionBuilder = Some(tempFunc2))
assert(catalog.listFunctions("db1", "*").map(_._1).toSet ==
Set(FunctionIdentifier("func1"),
FunctionIdentifier("yes_me")))
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 f39a3269ef..a91ad413f4 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
@@ -58,9 +58,8 @@ case class CreateFunctionCommand(
s"is not allowed: '${databaseName.get}'")
}
// We first load resources and then put the builder in the function registry.
- // Please note that it is allowed to overwrite an existing temp function.
catalog.loadFunctionResources(resources)
- catalog.registerFunction(func, ignoreIfExists = false)
+ catalog.registerFunction(func, overrideIfExists = false)
} else {
// For a permanent, we will store the metadata into underlying external catalog.
// This function will be loaded into the FunctionRegistry when a query uses it.
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
index b2d568ce32..6acac1a9aa 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
@@ -79,7 +79,7 @@ class CatalogSuite
val tempFunc = (e: Seq[Expression]) => e.head
val funcMeta = CatalogFunction(FunctionIdentifier(name, None), "className", Nil)
sessionCatalog.registerFunction(
- funcMeta, ignoreIfExists = false, functionBuilder = Some(tempFunc))
+ funcMeta, overrideIfExists = false, functionBuilder = Some(tempFunc))
}
private def dropFunction(name: String, db: Option[String] = None): Unit = {
diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala
index da87f0218e..0d0269f694 100644
--- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala
+++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala
@@ -161,7 +161,7 @@ private[sql] class HiveSessionCatalog(
FunctionIdentifier(functionName.toLowerCase(Locale.ROOT), database)
val func = CatalogFunction(functionIdentifier, className, Nil)
// Put this Hive built-in function to our function registry.
- registerFunction(func, ignoreIfExists = false)
+ registerFunction(func, overrideIfExists = false)
// Now, we need to create the Expression.
functionRegistry.lookupFunction(functionIdentifier, children)
}
diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/execution/benchmark/ObjectHashAggregateExecBenchmark.scala b/sql/hive/src/test/scala/org/apache/spark/sql/execution/benchmark/ObjectHashAggregateExecBenchmark.scala
index 73383ae4d4..e599d1ab1d 100644
--- a/sql/hive/src/test/scala/org/apache/spark/sql/execution/benchmark/ObjectHashAggregateExecBenchmark.scala
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/execution/benchmark/ObjectHashAggregateExecBenchmark.scala
@@ -221,7 +221,7 @@ class ObjectHashAggregateExecBenchmark extends BenchmarkBase with TestHiveSingle
val sessionCatalog = sparkSession.sessionState.catalog.asInstanceOf[HiveSessionCatalog]
val functionIdentifier = FunctionIdentifier(functionName, database = None)
val func = CatalogFunction(functionIdentifier, clazz.getName, resources = Nil)
- sessionCatalog.registerFunction(func, ignoreIfExists = false)
+ sessionCatalog.registerFunction(func, overrideIfExists = false)
}
private def percentile_approx(
--
GitLab