refactor: clean up loading of UDFs/UDTFs/UDAFs #3726
Conversation
| private <T> Map<String, T> createMap(Function<Integer, T> valueSupplier) { | ||
| Map<String, T> map = new HashMap<>(); | ||
| for (int i = 0; i < ENTRIES; i++) { | ||
| map.put(UUID.randomUUID().toString(), valueSupplier.apply(i)); |
There was a problem hiding this comment.
Add a test for null keys and null values. I see that you have an assertion for values to not be null but since the keys are optional schema, they could be null right? Also, there is not check in the actual class for null keys and values.
There was a problem hiding this comment.
Ooops sorry, will add them there
| private Range rangeUdf = new Range(); | ||
|
|
||
| @Test | ||
| public void shouldComputeIntRange() { |
There was a problem hiding this comment.
Is it an overkill to test for negative start and end?
| ], | ||
| "inputs": [ | ||
| { | ||
| "topic": "test_topic", "key": 1, "value": { |
There was a problem hiding this comment.
Add cases where key and value are null
There was a problem hiding this comment.
Hi @purplefox! It LGTM with one comment on handling null keys/values in the entries udf.
There was a problem hiding this comment.
🎉 thanks @purplefox! I like this patch :) (only reviewed the second commit, please merge the first one separately)
purplefox
force-pushed
the
func_load_refactor_8_new2
branch
from
724c0a5 to
efa27bd
Compare
This PR cleans up some of the function loading code, mainly replacing some overly complex streams based logic with simpler direct iteration.
Description
This PR is a copy of #3702 which has been closed due to a corrupt Jenkins workspace.
Please note this is a stacked PR. Please don't review changes from other PRs in the stack.
This PR cleans up the loading of functions. The code has been simplified by replacing grungy stream logic with good old fashioned for loops. I've also extracted a couple of other things out to new functions to aid clarity.
Testing done
Non functional change
Reviewer checklist