davidchaava commented on PR #28740: URL: https://github.com/apache/flink/pull/28740#issuecomment-5544255480
> @davidchaava checking where we are with this. Do you have a response on the > > * I raised isFlinkExtensionType() marker interface issue, @dalelane mentioned the three-item chain is tolerable in size. do you agree with Dale's feedback? > * Dale asked does this change accidentally alter leastRestrictive behaviour for the pre-existing RAW and BITMAP types? > Could you respond on this question please? > * Any chance you could look at this one @twalthr and @mxm? I would be more comfortable if one of you could approve this - if possible. Thanks for calling this out. I agree with Dale's feedback that introducing a marker interface for the current three Flink-specific types would be premature. The `leastRestrictive` change was intentionally scoped to `GEOGRAPHY`: identical types are resolved first; `LogicalTypeMerging.findCommonType` is used only when at least one input is `GEOGRAPHY`; all other cases still delegate to Calcite's `super.leastRestrictive`. Therefore, non-identical `RAW`-only and `BITMAP`-only inputs retain their previous behaviour. The implementation makes this explicit in three branches: https://github.com/apache/flink/blob/9b32af3fec7abc50544264cae45f2c02cece1400/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkTypeFactory.java#L267-L298 The corresponding tests cover these cases: `GEOGRAPHY` with `NULL` resolves to nullable `GEOGRAPHY`; incompatible `GEOGRAPHY`/`BITMAP` and `GEOGRAPHY`/`INT` combinations are rejected; and the existing `RAW` and `BITMAP` cases retain their expected results, including nullability and incompatible inputs. https://github.com/apache/flink/blob/9b32af3fec7abc50544264cae45f2c02cece1400/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/calcite/FlinkTypeFactoryTest.java#L256-L333 -- 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]
