[CALCITE-6823] Cannot convert CHAR to Integer when applying SubstitutionVisitor#4974
[CALCITE-6823] Cannot convert CHAR to Integer when applying SubstitutionVisitor#4974xuzifu666 wants to merge 7 commits into
Conversation
| /** Test case of | ||
| * <a href="https://issues.apache.org/jira/browse/CALCITE-6823">[CALCITE-6823] | ||
| * Cannot convert CHAR to Integer when applying SubstitutionVisitor</a>. */ | ||
| @Test void testDateFilter() { |
There was a problem hiding this comment.
What happens if the string is not a valid date literal?
You should get a decent error, not a crash.
There was a problem hiding this comment.
Make sense! I had fixed the issue and added a test about it.
| new DateString(requireNonNull(rexLiteral.getValueAs(String.class))) | ||
| .getDaysSinceEpoch()); | ||
| } catch (IllegalArgumentException e) { | ||
| LOGGER.warn("Invalid date literal: {}", rexLiteral.getValueAs(String.class), e); |
There was a problem hiding this comment.
Is NULL the correct result in these cases?
Is there a spec for what is supposed to happen?
Maybe you can add some documentation to the function about when it returns null.
Normally the program you are showing below in your test should use type coercion to insert a CAST of the string literal to date, and thus this case should probably be unreachable.
There was a problem hiding this comment.
Yes, in my view returning NULL is the correct behavior. According to the specification for getValue() method:
- When a value cannot be extracted or is invalid, it returns NULL;
- When NULL is returned, any containing optimization (such as materialized view substitution) is skipped;
- 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.
|
2 participants
jira: https://issues.apache.org/jira/browse/CALCITE-6823