924060929 commented on PR #67675: URL: https://github.com/apache/doris/pull/67675#issuecomment-6095321428
补充 FE 架构评审，基于当前 head `ff9e1b57c72db7584d0704812c887489c784ee07`。已有 glob 相关讨论不在这里重复。 Variant 的等值判断、哈希和排序需要使用一致的值语义。整体分层合理：BE 的 canonical 实现定义值语义，FE 类型分析决定 SQL 操作是否合法及如何转换类型，计划检查决定算子能否使用该类型，runtime filter 再单独判断其过滤语义是否适用。 ### 必须修改（仅针对本条架构评审） 没有确认新增的架构阻塞项。以下建议不要求本 PR 完成，也不表示已有 review 问题全部解决。本条为静态 FE 架构评审，不代表整个 PR 的合入状态或端到端执行验证。 当前几个关键修改位置是合适的： - 等值比较放在 `TypeCoercionUtils.processComparisonPredicateInternal`，让 SELECT、WHERE、JOIN 共用同一入口，比只在 JOIN 规则中放行合理。 - Variant RF 限制放在共用 `PushDownContext.isValid()`，覆盖下推入口；扫描端再次检查，是针对表达式经过 Project/SetOp/JOIN 替换后的校验。显式 CAST 成受支持标量后的 RF 仍可使用，这个边界合理。 - JSON 接入共用 coercion/signature 和 `ToJson`，现有 JSON builder 继续复用 `ToJson`，执行端通过 serde 转换，避免每个 JSON 函数各写一套转换逻辑。 ### 后续可以优化（非阻塞） 1. **按操作表达类型能力，避免一个笼统的“支持比较”。** 当前 `supportCompare()` 仍一律拒绝 Variant，等值比较通过前置特判绕过；但 Variant 实际上支持等值比较、哈希键和排序键，同时拒绝 SQL 大小比较与普通 RF。后续可先拆成几个小的、按操作区分的能力判断，并覆盖现有调用者。不要直接把 `supportCompare()` 中的 Variant 限制整体删除，否则可能意外扩大 IN 列表等其他操作的支持范围。无需为此引入庞大的通用框架。 2. **复用排序合法性判断。** `CheckAfterRewrite` 和 `EliminateOrderByKey.eliminateWindow` 都在判断排序键类型是否合法，新增类型支持时需要同步修改两处。后续可复用同一个判定，让删除冗余排序键的规则专注于优化逻辑。迁移检查前需要确认阶段顺序，不能直接删除早期检查。 3. **区分列存储配置和表达式的运行类型。** `ElementAt` 不再传播根列的 `predefinedFields`，方向合理：提取出的子路径值不应继续沿用根列的模板匹配语境。但只保留 max subcolumns、丢弃其他属性，也反映出当前 `VariantType` 同时承载列存储配置和表达式类型的职责。长期可以分离这两个概念，统一定义子路径结果应继承什么；本 PR 不必顺带重构整个类型系统。 4. **后续尽量按行为边界组织 PR。** 关系运算开放、全局 Variant→JSON 隐式转换、schema 自动 CAST 撤回，分别改变不同的用户行为，独立提交/PR 更方便审查、验证和回退。这里不要求将当前 PR 重新拆分。尤其 JSON coercion 是全局类型转换政策，后续变更时应继续覆盖所有相关签名消费者的重载选择、NULL 和嵌套类型，而不只验证几个 JSON 函数能够执行。 -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
