becketqin commented on code in PR #23313:
URL: https://github.com/apache/flink/pull/23313#discussion_r1332312205
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/factories/TestValuesTableFactory.java:
##########
@@ -1183,7 +1194,9 @@ private Map<Map<String, String>, Collection<Row>>
filterAllData(
for (Row row : allData.get(partition)) {
boolean isRetained =
FilterUtils.isRetainedAfterApplyingFilterPredicates(
- filterPredicates, getValueGetter(row));
+ filterPredicates,
+ getValueGetter(row),
+ Optional.of(getNestedValueGetter(row)));
Review Comment:
I think this is more of a programing convention for language. Optional is
equivalent to `@Nullable` annotation in Java.
The Optional parameter is kind of a paradox. If a parameter is Optional,
that means a null value is acceptable. So users can actually pass the nullable
value without wrapping it with Optional. If a parameter is not Optional, it
implies that the value cannot be null, so user need to make a null check on the
value. If it is null, likely an exception needs to be thrown, which is pretty
much what the invoked method will do when it sees null for the non-optional
value. So in either case, having an Optional parameter does not bring much
value, while introduce extra wrapping in the first case.
From a caller's perspective, a parameter value is known to be null or not,
if the value is going to be passed to the method call anyways, wrapping the
value with Optional is extra burden compared with passing the value directly.
On the other handle, the return value of the method call is unknown to the
caller. If the return value of a method call is an Optional, it explicitly
tells the caller that the return value needs to be checked because it might
return nothing. This is why Optional is useful for return value, but not the
parameters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
