HIVE-29838 - #6726
Conversation
Generated-by: Claude
asf-ci-hive
added
tests pending
tests passed
and removed
tests pending
tests passed
labels
1de2036 to
b8bf908
Compare
asf-ci-hive
added
tests failed
tests pending
tests passed
and removed
tests pending
tests failed
labels
|
Is JIRA ID correct ? Can't view it |
| return true; | ||
| } | ||
| String name = component.getName(); | ||
| if (UNTRUSTED_DENIED_CLASS_NAMES.contains(name)) { |
There was a problem hiding this comment.
this will disallow the three udfs globally, can them be configured from hive.server2.builtin.udf.blacklist, and checked within PartitionExpressionForMetastore.java
There was a problem hiding this comment.
IMO, HMS should not resolve HS2's configurations in most cases. Also, hive.server2.builtin.udf.blacklist is usually configured via SettableConfigUpdater. It is not easy to ensure that the class is invoked from HMS. Since these three classes aren't helpful for filtering out partitions, I'd say banning them doesn't introduce a new issue.
There was a problem hiding this comment.
Make sense, since this is only for HMS to prune partitions, Can we move the validation to PartitionExpressionForMetastore.java
There was a problem hiding this comment.
I have two reasons to prefer the current style. (1) The current implementation is robust since the fix is applied at the Kryo level; no Serde issue occurs. (2) I guess we can share the restriction with SARG.
| return true; | ||
| } | ||
| } | ||
| return false; |
There was a problem hiding this comment.
this might disable the customer class in their udf, why should we restrict the class as we can limit the udf to be used?
There was a problem hiding this comment.
Your point is understandable, but I don't have an effective way to restrict only what we really want to block.
One option is to disallow non-built-in UDFs as well. This could be reasonable. Does anyone want to extend the reflective partitioning filtering beyond standard UDFs? If not, we may remove L330-332.
There was a problem hiding this comment.
we can restrict the udf to be built-in UDFs, at least in HS2 the pushed predicate should be built-in functions(PartitionPruner#hasUserFunctions)
asf-ci-hive
added
tests pending
tests unstable
and removed
tests passed
tests pending
labels
|
4 participants
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?