fix(plan): harden sampled stats and join cardinality - #26791
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
matrix-meow
added
size/XL
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES
本次审查基于 base c72224b 与 head 43fb4cd，结论是当前实现仍不适合合入。
P1: ReCalcNodeStats 引入近似 O(J^3) 的重复遍历和大量分配
uniqueColsInSubtree 会递归遍历整棵 join 子树；同一个 INNER JOIN 又多次调用 getEquiJoinKeyPairs、唯一性、containment 等分析。优化流程还会重复执行多轮 ReCalcNodeStats。
同机、相同工具链的左深 join 基准：
- 20 表：base 0.31 µs / 0 alloc；PR 207 µs / 203 KB / 8,589 alloc
- 40 表：base 0.69 µs / 0 alloc；PR 1.59 ms / 2.34 MB / 61,234 alloc
- 80 表：base 1.89 µs / 0 alloc；PR 13.83 ms / 29 MB / 448,805 alloc
这不是微优化问题；复杂查询的规划阶段会额外产生数百 MB 临时分配。应在一次 join analysis 中缓存 tags、equality pairs、列集合和唯一性结果，并在统计重算中复用。
P1: 过滤复合唯一键的估算没有可证明的前置条件，仍可能低估 build side
estimateFilteredCompositeUniqueJoinCardinality 把 Stats.Selectivity 当作“保留复合键比例”。但 Stats.Selectivity 在 FILTER、JOIN 和子树中分别表示不同语义，不能证明键域覆盖率。
反例：unique composite side 从 100k 行过滤到 20k 行，probe 为 1m 行且最大单列 NDV=50k；如果 900k probe 行集中落在这 20k 个保留键上，实际 join 是 900k，而当前估算会从 400k 再截到 200k。该误差会影响 build/shuffle 决策并可能重新触发 #26742 的 HashBuild OOM。
此外，0.99 containment 阈值会产生不连续跳变：在 PR 测试参数下，probe max 从 1010 改为 1011 就可能从 1m 直接降到 1k。应移除基于全局选择率的复合键 cap；如果没有 tuple-domain/frequency 统计，应保留保守上界或保持 shuffle eligibility。
P2: 新增了一套与 determineHashOnPK 重复的 PK/唯一性传播逻辑
本 PR 的 uniqueColsInSubtree 与已有 determineHashOnPK 都在做 tags、equality keys、PK 等价传播。两套实现已经不一致：新实现声称支持 PROJECT，但没有把输出列映射回 ProjectList；已有实现也不会复用这套 lineage。
这会导致后续 join/projection 语义变更需要维护两套规则，同时也是当前重复遍历和性能退化的设计根因。建议抽取窄的只读 relational-key analysis，由 cardinality 和 HashOnPK 共同消费，不要继续增加第三套属性框架。
P2: PROJECT 唯一性和 NDV lineage 未闭环
Node_PROJECT 直接把 (RelPos, ColPos) 原样传给 child。普通 project 会产生新 binding tag，因此 scan tag=1 PK(k) -> project tag=2 output[0]=tag1.k 的唯一性证明会失败；getExprNdv 也无法通过 project tag 找到基础统计。该路径不会产生错误结果，但会让本 PR 的核心优化在常见子查询/CTE/别名投影下失效。
应仅对直接列投影做安全 lineage 映射；计算表达式保守拒绝，并补充 projection、重排、别名和 project-over-join 测试。
测试与维护性
新增约 647 行测试，其中 Q64 fixture 手工复制具体表、采样浮点数和当前 join 形状，并断言精确中间基数；这更像 incident snapshot，而不是稳定的不变量测试。TestFilteredCompositeUniqueJoinUsesRetainedTupleRatio 还没有真正进入对应 helper 分支。建议拆成小型 table-driven 边界/偏斜测试，再保留一个真实 planner 路径只验证 build side、shuffle 和语义结果，不冻结当前公式的精确浮点数。
建议先收敛为：单次 join analysis + 明确的 cardinality-local NDV 约束 + 可复用的 PK/lineage 属性；删除无法证明的选择率 cap 和重复的特例分支。
|
已按 review 收缩方案，当前最终 diff 只剩
未在本 PR 中继续猜测日期域相关性、复合键 overlap 或 tuple NDV。 |
matrix-meow
added
size/S
61bf54d to
4a4df16
Compare
aptend
changed the title
|
已用本地 SF1000 fixture 重新验证并替换上一版方案：不再修改 FILTER selectivity，也不改 cardinality 或 join order；最终 diff 仅在 shuffle admission 阶段为大型跨关系残余过滤保留输入侧风险上界。99 条 TPC-DS 中实质物理变化仅为 Q64 两个对称 join 开启 range shuffle；22 条 TPC-H 无物理变化。通用边界测试覆盖单关系、小型跨关系和大型跨关系三类。 |
4a4df16 to
dfe7e35
Compare
aptend
changed the title
matrix-meow
added
size/XL
4 participants
What type of PR is this?
Which issue(s) this PR fixes:
issue #26742
What this PR does / why we need it:
The optimizer currently consumes sampled object metadata as if every statistic had the same confidence. Invalid NDV, NULL, and sampled min/max combinations can then amplify cardinality errors, choose an unsafe build side, or create a skewed range shuffle.
This change makes statistics provenance explicit and fail-safe:
table_stats(..., 'patch', ...)atomically so malformed or ambiguous patches cannot become authoritative statistics;Validation after rebasing onto the current
main:go list,go build, andgo vetforpkg/pb/statsinfo,pkg/vm/engine,pkg/vm/engine/disttae,pkg/sql/plan, andpkg/sql/compile;The fixture comparison establishes that no structural regression was observed; it does not claim a runtime performance gain without actual-row and runtime measurements.