Repository: spark
Updated Branches:
refs/heads/master 9b377aa49 -> f7c145d8c
[SPARK-17996][SQL] Fix unqualified catalog.getFunction(...)
## What changes were proposed in this pull request?
Currently an unqualified `getFunction(..)`call returns a wrong result; the
returned function is shown as temporary function without a database. For
example:
```
scala> sql("create function fn1 as
'org.apache.hadoop.hive.ql.udf.generic.GenericUDFAbs'")
res0: org.apache.spark.sql.DataFrame = []
scala> spark.catalog.getFunction("fn1")
res1: org.apache.spark.sql.catalog.Function = Function[name='fn1',
className='org.apache.hadoop.hive.ql.udf.generic.GenericUDFAbs',
isTemporary='true']
```
This PR fixes this by adding database information to ExpressionInfo (which is
used to store the function information).
## How was this patch tested?
Added more thorough tests to `CatalogSuite`.
Author: Herman van Hovell <[email protected]>
Closes #15542 from hvanhovell/SPARK-17996.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/f7c145d8
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/f7c145d8
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/f7c145d8
Branch: refs/heads/master
Commit: f7c145d8ce14b23019099c509d5a2b6dfb1fe62c
Parents: 9b377aa
Author: Herman van Hovell <[email protected]>
Authored: Tue Nov 1 15:41:45 2016 +0100
Committer: Herman van Hovell <[email protected]>
Committed: Tue Nov 1 15:41:45 2016 +0100
----------------------------------------------------------------------
.../sql/catalyst/expressions/ExpressionInfo.java | 14 ++++++++++++--
.../sql/catalyst/analysis/FunctionRegistry.scala | 2 +-
.../spark/sql/catalyst/catalog/SessionCatalog.scala | 10 ++++++++--
.../spark/sql/execution/command/functions.scala | 5 +++--
.../org/apache/spark/sql/internal/CatalogImpl.scala | 6 +++---
.../org/apache/spark/sql/internal/CatalogSuite.scala | 15 ++++++++++++---
6 files changed, 39 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/f7c145d8/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionInfo.java
----------------------------------------------------------------------
diff --git
a/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionInfo.java
b/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionInfo.java
index ba8e9cb..4565ed4 100644
---
a/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionInfo.java
+++
b/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionInfo.java
@@ -25,6 +25,7 @@ public class ExpressionInfo {
private String usage;
private String name;
private String extended;
+ private String db;
public String getClassName() {
return className;
@@ -42,14 +43,23 @@ public class ExpressionInfo {
return extended;
}
- public ExpressionInfo(String className, String name, String usage, String
extended) {
+ public String getDb() {
+ return db;
+ }
+
+ public ExpressionInfo(String className, String db, String name, String
usage, String extended) {
this.className = className;
+ this.db = db;
this.name = name;
this.usage = usage;
this.extended = extended;
}
public ExpressionInfo(String className, String name) {
- this(className, name, null, null);
+ this(className, null, name, null, null);
+ }
+
+ public ExpressionInfo(String className, String db, String name) {
+ this(className, db, name, null, null);
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/f7c145d8/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
----------------------------------------------------------------------
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
index b05f4f6..3e836ca 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
@@ -495,7 +495,7 @@ object FunctionRegistry {
val clazz = scala.reflect.classTag[T].runtimeClass
val df = clazz.getAnnotation(classOf[ExpressionDescription])
if (df != null) {
- new ExpressionInfo(clazz.getCanonicalName, name, df.usage(),
df.extended())
+ new ExpressionInfo(clazz.getCanonicalName, null, name, df.usage(),
df.extended())
} else {
new ExpressionInfo(clazz.getCanonicalName, name)
}
http://git-wip-us.apache.org/repos/asf/spark/blob/f7c145d8/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
----------------------------------------------------------------------
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 3d6eec8..714ef82 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
@@ -943,7 +943,10 @@ class SessionCatalog(
requireDbExists(db)
if (externalCatalog.functionExists(db, name.funcName)) {
val metadata = externalCatalog.getFunction(db, name.funcName)
- new ExpressionInfo(metadata.className, qualifiedName.unquotedString)
+ new ExpressionInfo(
+ metadata.className,
+ qualifiedName.database.orNull,
+ qualifiedName.identifier)
} else {
failFunctionLookup(name.funcName)
}
@@ -1000,7 +1003,10 @@ 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.
- val info = new ExpressionInfo(catalogFunction.className,
qualifiedName.unquotedString)
+ val info = new ExpressionInfo(
+ catalogFunction.className,
+ qualifiedName.database.orNull,
+ qualifiedName.funcName)
val builder = makeFunctionBuilder(qualifiedName.unquotedString,
catalogFunction.className)
createTempFunction(qualifiedName.unquotedString, info, builder,
ignoreIfExists = false)
// Now, we need to create the Expression.
http://git-wip-us.apache.org/repos/asf/spark/blob/f7c145d8/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala
----------------------------------------------------------------------
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 26593d2..24d825f 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
@@ -118,14 +118,15 @@ case class DescribeFunctionCommand(
case _ =>
try {
val info =
sparkSession.sessionState.catalog.lookupFunctionInfo(functionName)
+ val name = if (info.getDb != null) info.getDb + "." + info.getName
else info.getName
val result =
- Row(s"Function: ${info.getName}") ::
+ Row(s"Function: $name") ::
Row(s"Class: ${info.getClassName}") ::
Row(s"Usage: ${replaceFunctionName(info.getUsage,
info.getName)}") :: Nil
if (isExtended) {
result :+
- Row(s"Extended Usage:\n${replaceFunctionName(info.getExtended,
info.getName)}")
+ Row(s"Extended Usage:\n${replaceFunctionName(info.getExtended,
name)}")
} else {
result
}
http://git-wip-us.apache.org/repos/asf/spark/blob/f7c145d8/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
----------------------------------------------------------------------
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
index f6c297e..44fd38d 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/CatalogImpl.scala
@@ -133,11 +133,11 @@ class CatalogImpl(sparkSession: SparkSession) extends
Catalog {
private def makeFunction(funcIdent: FunctionIdentifier): Function = {
val metadata = sessionCatalog.lookupFunctionInfo(funcIdent)
new Function(
- name = funcIdent.identifier,
- database = funcIdent.database.orNull,
+ name = metadata.getName,
+ database = metadata.getDb,
description = null, // for now, this is always undefined
className = metadata.getClassName,
- isTemporary = funcIdent.database.isEmpty)
+ isTemporary = metadata.getDb == null)
}
/**
http://git-wip-us.apache.org/repos/asf/spark/blob/f7c145d8/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
----------------------------------------------------------------------
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 214bc73..89ec162 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
@@ -386,15 +386,24 @@ class CatalogSuite
createFunction("fn2", Some(db))
// Find a temporary function
- assert(spark.catalog.getFunction("fn1").name === "fn1")
+ val fn1 = spark.catalog.getFunction("fn1")
+ assert(fn1.name === "fn1")
+ assert(fn1.database === null)
+ assert(fn1.isTemporary)
// Find a qualified function
- assert(spark.catalog.getFunction(db, "fn2").name === "fn2")
+ val fn2 = spark.catalog.getFunction(db, "fn2")
+ assert(fn2.name === "fn2")
+ assert(fn2.database === db)
+ assert(!fn2.isTemporary)
// Find an unqualified function using the current database
intercept[AnalysisException](spark.catalog.getFunction("fn2"))
spark.catalog.setCurrentDatabase(db)
- assert(spark.catalog.getFunction("fn2").name === "fn2")
+ val unqualified = spark.catalog.getFunction("fn2")
+ assert(unqualified.name === "fn2")
+ assert(unqualified.database === db)
+ assert(!unqualified.isTemporary)
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
