[CALCITE-7658] Type checker rejects CAST(ARRAY() AS ROW(x INT) ARRAY)#5104
Conversation
xuzifu666
left a comment
xuzifu666
left a comment
There was a problem hiding this comment.
LGTM, only left 2 minor comments.
| // without constraining the result. | ||
| // Note that UNKNOWN can be nullable, so this information is carried over to the result. | ||
| List<RelDataType> knownTypes = new ArrayList<>(types.size()); | ||
| // True if any UNKNONW type is nullable |
| } | ||
|
|
||
| /** UNKNOWN types in leastRestrictive() affect only the result nullability. */ | ||
| @Test void testLeastRestrictiveWithUnknown() { |
There was a problem hiding this comment.
The current test only covers a single UNKNOWN case; could we consider adding a test case that mixes multiple UNKNOWNs with known types, for example:
RelDataType r = f.typeFactory.leastRestrictive(
Lists.newArrayList(f.sqlUnknown, f.sqlUnknown, f.sqlBigInt));
assertThat(r.getFullTypeString(), is(f.sqlBigInt.getFullTypeString()));
This explicitly guarantees that any number of UNKNOWNs at any position will not affect the result.
Of course, this is just my understanding; feel free to take it into account.
There was a problem hiding this comment.
There should be very good coverage for non-UNKNOWN types, and this PR only reduces the case with UNKNOWN to the case without.
mihaibudiu
added
the
LGTM-will-merge-soon
|
There are currently two commits in this PR; please remember to squash them before merging. |
|
Well, I said above that the commits are independent, and could be merged separately. In other projects they would. |
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
|
2 participants
Jira Link
CALCITE-7658
Changes Proposed
I have left this PR as two commits, which could be merged separately. I could also file separate JIRA issues for them.
They both involve the handling of the UNKNOWN type, which is the type inferred for the elements of an empty array, for example.
The JIRA explains the rationale behind these changes.
Briefly:
ARRAY(ARRAY(), ARRAY(2)) should infer finally a type of ARRAY(INT) for the first array.
CAST(ARRAY() AS ROW(x INT) ARRAY) should be a legal program.