[SPARK-17996][SQL] Fix unqualified catalog.getFunction(...) #15542
Conversation
|
Test build #67161 has finished for PR 15542 at commit
|
|
Test build #67171 has finished for PR 15542 at commit
|
srinathshankar
left a comment
srinathshankar
left a comment
There was a problem hiding this comment.
Some minor comments
| return db; | ||
| } | ||
|
|
||
| public ExpressionInfo(String className, String name, String usage, String extended, String db) { |
There was a problem hiding this comment.
Nit: Could we put the db name before the expression name in the params ? Not a big deal if this requires more surgery.
| case _ => | ||
| try { | ||
| val info = sparkSession.sessionState.catalog.lookupFunctionInfo(functionName) | ||
| val db = if (info.getDb != null) info.getDb + "." else "" |
There was a problem hiding this comment.
Just inline the definition of val db into the definition of val name ? You don't use it anywhere else.
| description = null, // for now, this is always undefined | ||
| className = metadata.getClassName, | ||
| isTemporary = funcIdent.database.isEmpty) | ||
| isTemporary = metadata.getDb == null) |
There was a problem hiding this comment.
Is this still the right way to do this ? What about global temp tables ?
There was a problem hiding this comment.
We do not support global temp functions. So this is the right way to do it.
|
Test build #67883 has finished for PR 15542 at commit
|
|
Merging to master. Thanks for the review. |
## 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 <hvanhovell@databricks.com>
Closes apache#15542 from hvanhovell/SPARK-17996.
3 participants
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: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.