hive
hive copied to clipboard
HIVE-26259: Alter Function should also update resource uris
What changes were proposed in this pull request?
Add the alteration of resource uris for Function while calling alter_function
api.
Why are the changes needed?
Computing engines will load the jar of permanent UDF based on the resource uris, if we changed the resource uris through alter_function
api, which apparently won't work, we will not able to use this UDF because of the lack of jars.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Add an unit test:
mvn test -Dtest=org.apache.hadoop.hive.metastore.TestObjectStore#testFunctionOps -pl :hive-standalone-metastore-server
@pvary @ayushtkn Could you review this PR?
Do we have SQL support for this? If yes, then we might want to write a query test for it.
Also, please rebase, to fix the failing test.
Thanks, Peter
Hive SQL does not support ALTER FUNCTION
yet, we can create a new PR with this feature.
We should check around the security here. I just realized that changing the resource URIs could allow to inject malicious code under a function, so in most live clusters this should be rejected for regular users, and only admins should do this.
Hi @pvary thanks for your concern. It is indeed important to do authorization for operating UDF.
But I do not think it should be an admin privilege for the user level function, user should be able to create or alter their UDF, so an OWNER_PRIV
is enough just like #3360.
Also, we can achieve the authorization in Hive MetaStore by MetaStorePreEventListener
, I think it can be done after #3360 because there will be some similar code.
Hi @pvary thanks for your concern. It is indeed important to do authorization for operating UDF. But I do not think it should be an admin privilege for the user level function, user should be able to create or alter their UDF, so an
OWNER_PRIV
is enough just like #3360.
Makes sense
Also, we can achieve the authorization in Hive MetaStore by
MetaStorePreEventListener
, I think it can be done after #3360 because there will be some similar code.
I would prefer to have a security check already at hand when we enable the alter function.
I would prefer to have a security check already at hand when we enable the alter function.
OK, I will also add the security check here.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Feel free to reach out on the [email protected] list if the patch is in need of reviews.
@ayushtkn could you take a look again and check if there is any change required?