[SPARK-8883][SQL]Remove the OverrideFunctionRegistry #7260
Conversation
|
Merged build triggered. |
|
Merged build started. |
|
Test build #36662 has started for PR 7260 at commit |
|
Test build #36662 has finished for PR 7260 at commit
|
|
Merged build finished. Test FAILed. |
|
Merged build triggered. |
|
Merged build started. |
|
Test build #36688 has started for PR 7260 at commit |
|
Test build #36688 has finished for PR 7260 at commit
|
|
Merged build finished. Test PASSed. |
There was a problem hiding this comment.
wouldn't this throw UnsupportedOperationException when we register a UDF?
There was a problem hiding this comment.
No, it will not throws exception. HiveFunctionRegistry seems now is like a delegation, the function registry actually will apply to FunctionRegistry.builtin directly. And I think that's the original purpose of the master code. See UDFSuite.
There was a problem hiding this comment.
Maybe I'm missing something, but doesn't HiveFunctionRegistry just throw an exception?
override def registerFunction(name: String, builder: FunctionBuilder): Unit =
throw new UnsupportedOperationExceptionThere was a problem hiding this comment.
Yes, that's the tricky thing, as this code path is hidden(never be called) as we had the wrapper of OverrideFunctionRegistry, see code in HiveContext:
override protected[sql] lazy val functionRegistry: FunctionRegistry =
new OverrideFunctionRegistry(new HiveFunctionRegistry(FunctionRegistry.builtin))Since the OverrideFunctionRegistry is removed, we should update this part too.
UPDATE: I mean we should update the code of HiveFunctionRegistry.registerFunction.
|
Removing a class is not "minor". That should be reserved for things like fixing a spelling mistake in documentation. |
|
Sorry, @marmbrus , I am not so sure your mean, are you suggesting to create a associated jira issue or not removing the class for now? |
|
Please create a JIRA
|
chenghao-intel
changed the title
There was a problem hiding this comment.
can you add a unit test in hive to make sure function registration works.
There was a problem hiding this comment.
The unit test does exist in the master branch.
See: 'org.apache.spark.sql.hive.UDFSuite' and 'org.apache.spark.sql.UDFSuite'
|
Alright thanks. Merging this in. |
5 participants
Remove the
OverrideFunctionRegistryfrom the Spark SQL, as the subclasses ofFunctionRegistryhave their own way to the delegate to the right underlyingFunctionRegistry.