xuzifu666 commented on code in PR #4974:
URL: https://github.com/apache/calcite/pull/4974#discussion_r3322511948
##########
core/src/main/java/org/apache/calcite/plan/VisitorDataContext.java:
##########
@@ -150,9 +150,14 @@ public VisitorDataContext(@Nullable Object[] values) {
return Pair.of(index, rexLiteral.getValueAs(Integer.class));
case CHAR:
case VARCHAR:
- return Pair.of(index,
- new
DateString(requireNonNull(rexLiteral.getValueAs(String.class)))
- .getDaysSinceEpoch());
+ try {
+ return Pair.of(index,
+ new
DateString(requireNonNull(rexLiteral.getValueAs(String.class)))
+ .getDaysSinceEpoch());
+ } catch (IllegalArgumentException e) {
+ LOGGER.warn("Invalid date literal: {}",
rexLiteral.getValueAs(String.class), e);
Review Comment:
Yes, in my view returning NULL is the correct behavior. According to the
specification for getValue() method:
1. When a value cannot be extracted or is invalid, it returns NULL;
2. When NULL is returned, any containing optimization (such as materialized
view substitution) is skipped;
3. This is an explicit signal this optimization cannot be applied.
The spec for what is supposed to happen: the SQL validator should have
already coerced string literals to DATE type via CAST operations, making this
case unreachable. However, in practice, due to type inference rules or edge
cases in the validator, such cases can still slip through.
I added comments for cases where NULL is returned.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
