[FLINK-40355][table] Add new MAP_CONTAINS_KEY function - #28970
Conversation
b7b48ba to
fb849a3
Compare
| try { | ||
| return (boolean) equalityHandle.invoke(key, needle); | ||
| } catch (Throwable t) { | ||
| throw new FlinkRuntimeException(t); |
There was a problem hiding this comment.
In the past we have not wanted to issue errors in SQL as it would end the job. I suggest we return false and log in this error case. In the error case they are not equal.
There was a problem hiding this comment.
The catch Throwable here mirrors other builtin function's implementation, functions like :
- ARRAY_CONTAINS
- ARRAY_POSITION
- ARRAY_REMOVE
I was also talking to my mentor about logging and how it is not great in streaming as we are processing millions of records and logs can get full really quickly and how you dont know easily what error causes a specific entry in a log.
Please let me know your thoughts, intrested to hear more, I see that there are two options, either keep this implementation or alter other functions aswell to ensure that we are returning False as you say and logging. However if we decide to do this to the naked eye some invalid input that would cause an error would be hidden by a silent failure only to be discovered in a log.
Would love to hear you thoughts regarding this.
Thanks
Vas.
| description: | | ||
| Returns TRUE if the given key exists in the map, FALSE otherwise. Returns NULL if the map is | ||
| NULL. A NULL key matches a NULL key in the map. The given key is cast implicitly to the map's | ||
| key type where Flink's implicit casting rules allow it; otherwise the call fails validation. |
There was a problem hiding this comment.
when we say fails validation - I assume we should return false as per my other comment
There was a problem hiding this comment.
This validation error is called during the planning stage i think when a map and a key is given with different types on the keys
3 participants
What is the purpose of the change
This pull request add a new function maoContainsKey function, which which given a key which check if it is present within a map.
Brief change log
Verifying this change
Please make sure both new and modified tests in this PR follow the conventions for tests defined in our code quality guide.
This change added tests and can be verified as follows:
/mvnw -o -pl flink-table/flink-table-planner -Dtest='JsonFunctionsITCase' -Dsurefire.failIfNoSpecifiedTests=false -Dcheckstyle.skip=true -Dspotless.check.skip=true -Drat.skip=true -e -Denforcer.skip=true test
(example:)
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Opus 5