hive icon indicating copy to clipboard operation
hive copied to clipboard

HIVE-26259: Alter Function should also update resource uris

Open wecharyu opened this issue 2 years ago • 7 comments

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

wecharyu avatar May 23 '22 17:05 wecharyu

@pvary @ayushtkn Could you review this PR?

wecharyu avatar May 29 '22 06:05 wecharyu

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

pvary avatar May 31 '22 05:05 pvary

Hive SQL does not support ALTER FUNCTION yet, we can create a new PR with this feature.

wecharyu avatar May 31 '22 13:05 wecharyu

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.

pvary avatar Jun 27 '22 09:06 pvary

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.

wecharyu avatar Jun 27 '22 11:06 wecharyu

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.

pvary avatar Jun 27 '22 11:06 pvary

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.

wecharyu avatar Jun 27 '22 12:06 wecharyu

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.

github-actions[bot] avatar Aug 30 '22 00:08 github-actions[bot]

@ayushtkn could you take a look again and check if there is any change required?

wecharyu avatar Dec 20 '23 13:12 wecharyu