eldenmoon commented on PR #69995: URL: https://github.com/apache/doris/pull/69995#issuecomment-6581204870
## Review：兼容性、性能、可维护性与冗余 审查提交：`ee6ce712a0381422bc44e4d8d27490cf66f32464`；比较基线为 merge base `49bfbdecffb5b37aeadb0982700533e8daf7768d`。 按 `doris-repo-review` 工作流进行了两轮静态交叉审查，覆盖全部 59 个变更文件及相关调用链。**结论：REQUEST_CHANGES，1 项 Major、2 项 Minor。建议先修复 F-01。** 本轮未编译、未运行测试；下列触发场景与影响来自源码追踪，PR 描述中的测试结果不作为本轮实测。 ### F-01 · Major：类型 SQL 往返丢失显式 `nested_group=false`，使合法表的 CREATE TABLE LIKE 失败 位置：[catalog VariantType.toSql](https://github.com/apache/doris/blob/ee6ce712a0381422bc44e4d8d27490cf66f32464/fe/fe-type/src/main/java/org/apache/doris/catalog/VariantType.java#L208-L221)、[Nereids VariantType.toSql](https://github.com/apache/doris/blob/ee6ce712a0381422bc44e4d8d27490cf66f32464/fe/fe-core/src/main/java/org/apache/doris/nereids/types/VariantType.java#L209-L222)。 两处 serializer 会输出 `variant_escape_dots_in_keys=true`，但只在 nested group 为 true 时输出该属性。对新功能而言，显式的 `variant_enable_nested_group=false` 是合法配置的重要组成部分，省略它会让重新解析依赖当前会话默认值。 具体场景： 1. 设置 `default_variant_enable_nested_group=true`。 2. 创建源表，Variant 列显式配置 `variant_escape_dots_in_keys=true`、`variant_enable_nested_group=false`。 3. 执行 `CREATE TABLE dst LIKE src`。 `CREATE TABLE LIKE` 经 `Env.getCreateTableLikeStmt → Column.toSql → VariantType.toSql` 生成 SQL，再在当前会话中重新解析。生成的 SQL 缺少 nested=false，因此 [LogicalPlanBuilder](https://github.com/apache/doris/blob/ee6ce712a0381422bc44e4d8d27490cf66f32464/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java#L5453-L5484) 从会话补成 true，最终因 escape=true 与 nested=true 的互斥检查而拒绝。在该会话下重放 SHOW CREATE 输出也有相同问题。这里确认的是合法 DDL 无法往返，没有证据表明会损坏已有数据。 **建议：** 两处 SQL renderer 都在 `enableNestedGroup || escapeDotsInKeys` 时显式输出 nested group 的实际布尔值。开启 escaping 时必须保留 false。补上“会话默认 true + 源列显式 false”的类型解析往返、CREATE TABLE LIKE 和 SHOW CREATE 重放测试，同时保留旧模式的序列化基线。 ### F-02 · Minor：compaction 的预校验会在 segment cache miss 时重复初始化 Segment / column reader 位置：[variant_util.cpp:764–770](https://github.com/apache/doris/blob/ee6ce712a0381422bc44e4d8d27490cf66f32464/be/src/exec/common/variant_util.cpp#L764-L770)，以及后续 [schema 汇总加载路径](https://github.com/apache/doris/blob/ee6ce712a0381422bc44e4d8d27490cf66f32464/be/src/exec/common/variant_util.cpp#L835-L864)。 新校验阶段加载 segment 并初始化 root reader，但局部 handle 在该 rowset 检查结束后释放。未命中 segment cache、且不向该缓存插入的路径上，后续 schema 汇总又加载同一批 segment/root reader，产生重复的对象和元数据初始化。 **建议：** 让校验与 schema 汇总共用已加载的 handle，或在汇总遍历中完成校验；没有 schema 汇总的 compaction 模式仍须保留校验。 这里不能把 marker 校验本身删掉：缓存 reader 之前校验的是所属 rowset 的 schema，目标 compaction schema 仍须独立验证。也不能直接声称磁盘 I/O 翻倍：独立的 footer/page cache 可能避免重复读取，热 segment cache 也可复用对象。本项仅报告源码可证明的重复初始化，未测量耗时退化。 ### F-03 · Minor：显示名仍合并两种逻辑路径，导致部分 CAST 标量下推和剪枝能力丢失 位置：[olap_scanner.cpp:639–642](https://github.com/apache/doris/blob/ee6ce712a0381422bc44e4d8d27490cf66f32464/be/src/exec/scan/olap_scanner.cpp#L639-L642)。 BE 最终改用 storage column name 是必要的，但上游 FE 仍把 `['a.b']` 与 `['a','b']` 都展开为显示名 `v.a.b`，[CAST 收集](https://github.com/apache/doris/blob/ee6ce712a0381422bc44e4d8d27490cf66f32464/be/src/exec/operator/scan_operator.cpp#L1136-L1178)也仍按该名称分组并要求恰好一个 CAST。新功能已经把它们保存为不同物理子列，这层分组身份却没有同步区分。 例如在两个路径均已提取为 BIGINT 子列的 escaped 表上，同时使用： ```sql CAST(v['a.b'] AS BIGINT) = 1 AND CAST(v['a']['b'] AS BIGINT) = 2 ``` 两条路径的 CAST 会混在同一组，不能分别进入标量 range 路径；当父 Variant 配置 Bloom filter、两个已提取子列具备相应 Bloom 索引时，会损失这条标量谓词路径的 Bloom 剪枝机会。 **影响边界：** slot ID 检查仍阻止谓词被错误应用到另一列；common-expression fallback 仍可使用倒排索引和 zone map。因此本项是可选的性能改进，不是已确认的错误结果，也不等于所有索引失效或必然全表扫描。 **建议：** CAST 收集与 range 初始化按 slot ID 保持身份，再从各 slot 的完整路径生成 storage name；补一条同时引用 dotted literal 与 nested path 的组合用例，检查两条标量下推路径都能建立。 ### 兼容性与冗余结论 - 本轮没有确认默认关闭时的旧表行为回归；以上三项均为新 opt-in 功能上的缺口或优化机会，`Regression: no`。 - `% → %25` 与 `. → %2E` 的组合、持久化 marker、property/marker 一致性检查，以及 escaped sparse key 的字节排序都有实际用途，应保留。尤其稀疏排序服务于读取端二分查找，不是多余排序。 - FE 中 parser、直接构造的 CTAS ColumnDefinition、CREATE/ALTER INDEX 的校验覆盖不同入口，不能因条件相似就直接去掉其中一处。 - 优先简化的是 **F-02 的重复加载/初始化**，以及 **F-03 分散的显示名/存储名身份转换**。 - 混合版本启用和降级仍属于 PR 明确排除的使用方式。新属性应在全部 FE/BE 升级后启用；本次没有把可选字段的默认值兼容等同于混合版本安全。 完整覆盖已在第二轮收敛；没有确认新的数据丢失、错误结果或旧模式回归。建议修复 F-01 后再合入，F-02/F-03 可作为后续性能与维护性改进。 -- 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]
