[englefly]

History-based optimizer statistics for the nereids planner

What problem does this solve?

The nereids optimizer estimates a plan from catalog statistics alone, so a query whose data distribution
or predicate selectivity the catalog cannot describe (a skewed join key, a selective range, a filter
whose matching rows are known from previous executions) is estimated from heuristics.

This PR adds a history-based optimizer (HBO):

• Plan identity. A plan node is identified by the struct info of its memo group: a tree description
  of the node kinds, the normalized expressions, the scan identities and the children. Two literal modes
  are keyed: the exact one and the constant agnostic one which folds the literals of a predicate. The key
  is the sha256 of the structural identity of that tree (length-prefixed fields), not of its printed
  form, so a table name, an alias or a literal which carries a character of the printed grammar cannot
  stand for another field. The scan token records the table id, the relation alias, the index and the
  selected partitions - the constant agnostic key is then only applied to the same data state.
• Read side. A learned entry is matched by fingerprint and validated against the input table
  statistics it recorded (rows read per scan) and against the current data state (visible version, row
  count, partitions), so a stale or drifted entry is not applied. A published entry carries the
  predicates of the scans it measured, the grouping definitions and the join conditions, so two queries
  which differ in a predicate the key folds cannot share a count.
• Manual management. HBO SET/SHOW/DELETE STATISTICS (pinned entries), HBO SET LEARNED STATISTICS
  (tests and experts), hbo_persist_pinned_to_internal_db (pinned entries survive a restart, with
  generation-bounded deletions), and SET show_hbo_fingerprint prints the key and the struct of every
  node in EXPLAIN together with the verdict of the pinned and learned entries.

Release note

• Learned statistics are published after qualifying executions and reused by later queries with the same
  plan; pinned statistics can be managed with the HBO ... STATISTICS statements, persisted to
  __internal_schema, and EXPLAIN can print the plan key together with the statistics which were
  applied.

Check List (For Author)

• Test: Unit Test + Regression test
  • Unit: the org.apache.doris.nereids.stats package - struct-info golden corpus (76 plan cases, 51
    text cases, regenerated and asserted), scan identity, freshness, learned guards, pinned race,
    expansion keys, input-table merge - plus the HBO command suites: 200 cases pass.
  • Regression: a FE rebuilt from this branch runs query_p0/hbo to its baseline (19 suites; the 8
    legacy/* suites fail because the environment does not support them). The injection, drift,
    learned, persistence and display suites pass.
• Behavior changed: Yes - the optimizer reuses history for a matched plan, and EXPLAIN can print the key
  of a node with SET show_hbo_fingerprint=true.
• Does this need documentation: Yes - the HBO statements, the session variables and the internal table
  need a manual.

Note

This PR is the squashed version of #67549 (one commit per branch tip instead of the incremental history),
opened with the same tree so the review can be done on a single change.

Known limitation: the join identity is bound to the relation aliases

Two queries which differ only by the names of their aliases get different keys:

select * from t1 as a, t2 as b, t3 as c where a.id = b.id and b.name = c.name
select * from t1 as x, t2 as y, t3 as z where x.id = y.id and y.name = z.name

Their fingerprints differ (a0424a5c… against 97991fdf… on a local cluster), because a join condition
is rendered with the alias qualifier of its slots and every scan token carries the alias of its relation
(#ra:). A pinned or learned entry is therefore alias sensitive: renaming an alias invalidates the
entries of the plan, and they have to be pinned again.

That is deliberate, and it is the safe direction: an alias is the only occurrence identity a plan
provides. A table can appear more than once, and the occurrences play different roles, which the join
conditions alone cannot tell apart - normalizing the operands to their table and column is not enough,
even for an inner join:

-- the same normalized conditions ({t.id = t.id, t.x = t.pid}) but a different row count
select * from t a join t b on a.id = b.id join t c on a.x = c.pid;
select * from t a join t b on a.id = b.id join t c on b.x = c.pid;

An occurrence index derived from the tree would be stable for neither of the two call sites (a group is
estimated with GroupPlan children, while EXPLAIN prints the chosen plan), so it cannot replace the
alias either.

Follow-up (not part of this PR): make the key alias insensitive with a canonical labelling of the join
graph - instance nodes labelled by their table, edges labelled by the columns and the operator, refined
deterministically into a canonical numbering. Automorphic instances then share a label, which is
harmless for a row-count estimate (swapping them only transposes the join output), while instances with
different roles (the two queries above) are distinguished. Until then, the alias stays part of the
identity.

[hello-stephen]

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?

[englefly]

run buildall

[englefly]

/review

[github-actions]

Static review of all supplied changed paths at 1322847. Eight new P2 findings cover incomplete semantic keys, cross-table inner-join expansion collisions, selective/zero input baselines, strict learned freshness, stale skew costing, and learned placeholder replacement. No existing inline findings were supplied. No additional user focus was specified.

Critical checkpoint conclusions:

• Goal and evidence: HBO identity, feedback reuse and manual management are implemented, but the inline findings prevent reliable reuse in the concrete plans described. Added tests cover many identity and injection cases; they do not establish automatic feedback reuse.
• Scope and reuse: the broad change is related to HBO; group descriptions and providers centralize the main lifecycle. Semantic fields and parallel consumers need consistent treatment.
• Concurrency: planner, profile, admin, config and loader threads share caches. Pinned mutation/load operations serialize on pinnedLoadLock, including internal SQL. Planning reads concurrent caches without acquiring that lock; no concrete deadlock was found.
• Lifecycle: per-query snapshots and cache expiry bound retained feedback. Loader restore is one-time and best effort per FE. The learned-placeholder replacement defect is described inline; no native/static initialization issue applies.
• Configuration: mutable ratio, guard and persistence settings are read on use; pinned cache capacity is explicitly startup-only. Strict learned-version behavior is defective as reported.
• Compatibility: no new FE-BE wire field or user-data format changes. Old learned keys cold-miss; internal pinned table columns are checked and old schemas explicitly require recreation.
• Parallel paths: filters, joins, aggregates, Java/Python UDF metadata and physical reconstruction were checked. Broadcast skew costing lacks the freshness fence used by row-count lookup. Mark-join semantics require a distinct key.
• Conditional checks: unsupported CTE consumers, table sampling, external feedback and physical-only partial stages are deliberately excluded; no duplicate local/final aggregate issue was verified.
• Tests: unit/golden and regression coverage was inspected, including negative command/guard cases. Real profile publication/reuse, restart loading and concurrent internal SQL lack end-to-end coverage. No builds or tests were run under this static-only contract.
• Test results: the added persistence output agrees with its injected values and singleton queries. Eight legacy suites are unchanged-content renames. Output-generation provenance was not independently executed.
• Observability: EXPLAIN applied/skipped annotations and persistence/load/snapshot logs support diagnosis; no separate logging defect was verified.
• Persistence and failover: pinned records use internal OLAP SQL, with checked execution errors, generation-bounded deletes and pending-load tombstones; cross-FE live refresh is explicitly unsupported. Restart/failover behavior remains unexecuted.
• Data writes and atomicity: no user-data write or transaction visibility/delete-bitmap path changes. Pinned memory operations are serialized and database persistence is declared best effort; no additional concrete atomicity bug was found.
• FE-BE propagation: existing runtime counters/statistics transport are reused; the counter-to-population mismatch is reported inline.
• Performance and memory: caches, scan-width gating and relation prefilters limit common-path work. Incorrect estimate reuse/misses and stale skew discounts can regress plans; no additional substantiated performance issue was found.
• Other review concerns: all candidates and initial risks have explicit dispositions in the shared ledger. Findings concern estimates, costing and feature behavior; no wrong-result claim is made.

Convergence is recorded separately after reviewer validation of the frozen findings. Validation limits above describe static review scope, not executed runtime results.

Review converged after 3 round(s).

[github-actions]

[P2] Preserve semantic fields in normalized expressions

Aggregate(gb=CAST(t.s AS DECIMAL(10,0))) -> Scan(t) and the same plan with DECIMAL(10,1) both encode A{gb:Cast(col(t.s))}. For string values '1.1' and '1.2' these have one versus two groups. Cast.targetType is outside its children, so this fallback drops it in both literal modes; computeAggregate can then reuse the other query's learned or pinned count while identical scans pass freshness. The same omission drops OrderExpression direction: HAVING group_concat(v ORDER BY v ASC)='a,b' and DESC share an exact key even though only one group passes. Include cast targets and order direction/null ordering in expression identity, and cover these paired plans.

[englefly]

不需要. 出现cast 绝大部分都是因为 两个decimal字段的 scale/precision不匹配.如果cast的type不同,说明参与计算的两个字段至少一个不同. 所以两个hbo条目本身就不可能匹配

[github-actions]

[P2] Include the bound tables in inner-join expansion keys

FROM t1 a JOIN t2 b ON a.k=b.k and FROM t3 a JOIN t4 b ON a.k=b.k in the same database retain the same alias-qualified condition, even after alias-only projects are eliminated. During estimation this traversal finds no relations beneath GroupPlan leaves. In the concrete plan, bound slot qualifiers are internal.db.a/internal.db.b, but the lookup map only indexes bare aliases and relation.getQualifier() (catalog/database), so those bindings are empty there too. Both joins consequently get the same expansion fingerprint despite different tables and fan-outs; an expansion pinned for the first is applied to the second without a freshness guard. Resolve stable table identities through memo groups and index the full bound qualifiers consistently with EXPLAIN.

[englefly]

Implemented in the new commit of this branch (hbo-structinfo-squashed):

• An expansion key now requires every operand of the join conditions — the equality pairs and the residual conditions — to be a direct slot of a catalog table, i.e. SlotReference.getOriginalTable() and getOriginalColumn() present. The operand identity is qualifiedNameOf(originalTable) + "." + originalColumn.getName(), read from the slot itself, so it is identical while the group is estimated (GroupPlan children) and while the chosen plan is printed, and it does not depend on the alias or on the qualifier shape (internal.db.a).
• The key is the structural hash of: the join type, the sorted equality pairs rendered with those operand identities (EqualTo(col(internal.db.t1.k),col(internal.db.t2.k))), the sorted residuals (literals kept) and the sorted operands:[...]. Duplicate alias bindings, subtree traversal and relation lookups are gone.
• A join whose operand is not a table column (cast, aggregate output, derived alias) has no key: canonicalOf/fingerprintOf return empty, so EXPLAIN prints no join type=join_expansion line and no lookup can match it. That is the fail-closed behaviour you asked for instead of a colliding key.
• An outer join additionally names the columns of each side (sides:[...], read from the child output slots), so A LEFT JOIN B and B LEFT JOIN A stay distinct while remaining stable across the two call sites. Inner joins stay commutative.
• Tests: HboJoinConditionsTest now pins t1 AS a JOIN t2 AS b vs t3 AS a JOIN t4 AS b (different keys), a computed operand (no key), the outer-join side, the residual literals and that the key is not the sha256 of the canonical text. The expansion regression suite reads its fingerprint/struct from the EXPLAIN annotation and has not been re-run on a FE rebuilt from this commit yet; I will report the result once it is.

[github-actions]

[P2] Keep value-defining literals in set-operation mappings

These mappings define branch output values, so folding their literals can change distinct cardinality. For SELECT a AS x FROM t UNION SELECT a % N AS x FROM u, BuildAggForUnion produces Aggregate(gb=x) -> UnionAll(Project(t.a), Project(u.a % N)). With t={0} and u=0..999, N=2 yields two groups and N=1000 yields 1000, but NO_LITERAL encodes both mappings as Mod(col(u.a),lit(*)). The aggregate's learned key therefore collides, and unchanged scans with no predicates pass both matching and freshness. Preserve mapping literals in both modes, as grouping definitions already do; the existing computed-branch golden pair currently records this collision.

[englefly]

不需要. NO_Literal 模式本身就意味着常熟部分的差异会带来行数误差, 注册NO_LITERAL hbo 就意味着用户认为可以忽略这个误差

[github-actions]

[P2] Compare learned inputs against the same row population

Filter(k=1) -> Scan(t) on 100000 sorted distinct keys can physically read one row: storage key ranges prune the row bitmap before raw_rows_read/ScanRows are incremented. RuntimeProfile now records that counter, but the live descriptor contains all 100000 selected-partition rows. An immediate identical query therefore compares 1 with 100000, calls its fresh feedback stale, and cannot reuse the measured filter or parent join/aggregate count. Zero reads have the opposite failure: recorded == 0 is still skipped as unmeasured, so an absent-key result survives insertion of matching rows. Snapshot a comparable input-population baseline and distinguish missing measurements from measured zero.

[englefly]

learned HOB 是老版本的功能, 且老版本里已经是不可用状态. 不在这个pr里处理

[github-actions]

[P2] Enforce strict version matching for learned entries

With hbo_row_count_change_ratio=0, this branch still declares equal input counts live without checking a recorded visible version. For Aggregate(gb=t.v) -> Scan(t), learn one group from 1000 identical values, then update those values to many distinct values. After compaction and row-count refresh the population can again be 1000 while the visible version has advanced. The new version-free key and unchanged scan predicates match, so applyLearnedStats applies the obsolete one-group estimate despite strict mode. ScanPlanStatistics retains a scan reference but no immutable version snapshot. Record the learned input versions and require equality in nonpositive-tolerance mode, as the pinned path does.

[englefly]

learned hbo 的问题是老版本的遗留问题, 不在这个pr处理

[github-actions]

[P2] Check learned freshness before applying the skew discount

This parallel consumer now uses a version-free key but calls getMatchedPlanStatistics, which checks predicates/runtime-filter safety without learnedEntryFreshness. After a skewed join is learned, substantial uniformly distributed growth can exceed the input-row tolerance and eliminate that skew. HboStatsCalculator.applyLearnedStats rejects that run as stale, yet broadcast costing still reads its old skew ratio and multiplies probe/build work by 0.1, potentially choosing an inappropriate broadcast plan. Previously the embedded visible version prevented this hit. Retrieve the matched entry and validate its input freshness before using its skew measurements.

[englefly]

learned hbo 的问题是老版本的遗留问题, 不在这个pr处理

[github-actions]

[P2] Let actual feedback replace an injected learned placeholder

After HBO SET LEARNED STATISTICS for a join's NO_LITERAL key, the cache contains an empty-input wildcard. The first actual profile publication does not replace it: Profile.updatePlanStatistics cannot FULL_MATCH its zero inputs with the real nonempty input list and appends the measurement. Later identical executions replace only that measurement, leaving [wildcard, measured] below the default ten-entry limit indefinitely. This loop always returns the wildcard, so its arbitrary row count keeps overriding real feedback and bypassing drift checks. Remove the placeholder when actual same-key feedback arrives, or prefer a matching measured entry once one exists; otherwise LEARNED behaves like a persistent pinned override.

[englefly]

手动注入learned条目的场景是测试场景,生产环境不会用手动注入learned 条目

[github-actions]

[P2] Distinguish marking joins from filtering semi joins

Project(t.k, mark AS m) -> LEFT_SEMI_JOIN(mark=true, t.k=u.k) -> [Scan(t), Scan(u)] preserves every t row, while the same semi join without the marker filters unmatched rows. For nonnullable keys, projected t.k IN (SELECT k FROM u) produces the first plan; an ordinary semi join produces the second. With t={1,2}, u={1}, they output two versus one rows, but this key writes only join type and merged conditions, omitting isMarkJoin(). Both literal modes collide, and computeJoin can apply the other plan's learned/pinned count because their input scans are identical. Encode marking semantics in the join identity and cover both plans.

[englefly]

GroupStructInfo 增加属性记录join是否是mark

[englefly]

Fixed in the branch ([fix](fe) Include the mark-join flag in the join identity):

• GroupStructInfo.Node now carries markJoin (from Join.isMarkJoin()), the canonical writes it as J{<type>,mark,c:[...]} and the structural key writes it as a field, so Project(mark AS m) -> LEFT_SEMI_JOIN(mark=true, ...) and the plain LEFT_SEMI_JOIN over the same scans no longer share a key in either literal mode.
• Two corpus cases pin the pair (join_mark_semi / join_plain_semi) and a test asserts the keys differ in both modes and that the canonical writes the flag.
• Note: because the flag is a field of the join identity, the fingerprints of all join cases moved once (the regenerated corpus shows that change explicitly).
• Tests: org.apache.doris.nereids.stats.* + the HBO command suite, 200 cases pass. The regression suites were not re-run against a FE rebuilt from this commit.

[hello-stephen]

FE UT Coverage Report

Increment line coverage 50.84% (1735/3413) 🎉
Increment coverage report
Complete coverage report

[hello-stephen]

TPC-H: Total hot run time: 28773 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 1322847012729a1e190651ebff44add931369c47, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17605	5155	5074	5074
q2	2167	319	263	263
q3	10167	1414	829	829
q4	4680	469	329	329
q5	7455	815	542	542
q6	177	181	154	154
q7	757	797	622	622
q8	9253	1332	1080	1080
q9	5505	4203	4168	4168
q10	6799	1287	1026	1026
q11	431	249	230	230
q12	633	391	277	277
q13	18163	3755	3025	3025
q14	272	262	246	246
q15	q16	735	716	668	668
q17	1216	770	574	574
q18	7141	6186	6131	6131
q19	1249	967	585	585
q20	384	336	236	236
q21	5828	2431	2493	2431
q22	378	333	283	283
Total cold run time: 100995 ms
Total hot run time: 28773 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	6022	5914	6130	5914
q2	717	601	596	596
q3	5553	5794	5375	5375
q4	2037	2141	1604	1604
q5	5651	5546	5752	5546
q6	243	187	144	144
q7	2228	1943	1790	1790
q8	2987	2568	2613	2568
q9	7935	7936	7811	7811
q10	3699	3590	3119	3119
q11	578	402	389	389
q12	660	661	506	506
q13	3296	3554	3023	3023
q14	298	285	275	275
q15	q16	663	688	612	612
q17	7794	7067	6907	6907
q18	12990	12198	12946	12198
q19	887	773	792	773
q20	2144	2156	1925	1925
q21	5614	4619	4766	4619
q22	483	462	411	411
Total cold run time: 72479 ms
Total hot run time: 66105 ms

[hello-stephen]

TPC-DS: Total hot run time: 151139 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 1322847012729a1e190651ebff44add931369c47, data reload: false

query5	4320	602	450	450
query6	444	206	197	197
query7	4823	453	223	223
query8	328	167	149	149
query9	8793	3161	3199	3161
query10	489	315	254	254
query11	6041	3851	3571	3571
query12	153	90	83	83
query13	1249	445	320	320
query14	6100	3950	3686	3686
query14_1	3393	3421	3500	3421
query15	210	198	195	195
query16	956	423	408	408
query17	871	667	530	530
query18	2449	410	298	298
query19	214	155	130	130
query20	86	82	80	80
query21	213	122	105	105
query22	13004	12951	12770	12770
query23	13648	12831	12343	12343
query23_1	12495	12511	12495	12495
query24	7359	831	460	460
query24_1	468	474	468	468
query25	524	391	329	329
query26	1252	254	134	134
query27	2779	441	277	277
query28	4608	1985	2013	1985
query29	1545	594	431	431
query30	315	227	180	180
query31	1001	940	807	807
query32	182	90	94	90
query33	519	293	237	237
query34	894	840	496	496
query35	754	763	639	639
query36	818	805	718	718
query37	134	95	85	85
query38	1775	1739	1688	1688
query39	806	796	736	736
query39_1	736	739	742	739
query40	224	111	96	96
query41	70	64	63	63
query42	89	81	81	81
query43	342	362	311	311
query44	1343	686	690	686
query45	175	172	165	165
query46	839	962	572	572
query47	2834	2909	2866	2866
query48	297	312	225	225
query49	592	402	302	302
query50	690	282	215	215
query51	10219	10249	10386	10249
query52	87	85	75	75
query53	193	208	157	157
query54	261	220	198	198
query55	93	70	74	70
query56	292	219	209	209
query57	1636	1633	1562	1562
query58	329	276	260	260
query59	2085	2086	1915	1915
query60	280	243	227	227
query61	153	146	145	145
query62	394	345	302	302
query63	191	162	166	162
query64	2734	960	748	748
query65	3292	3261	3254	3254
query66	1781	422	309	309
query67	20544	20171	20020	20020
query68	2941	992	606	606
query69	415	282	241	241
query70	848	787	766	766
query71	314	224	208	208
query72	2854	2745	2436	2436
query73	520	541	301	301
query74	4679	4543	4356	4356
query75	2353	2282	1922	1922
query76	2327	1030	632	632
query77	356	384	289	289
query78	9725	9813	9275	9275
query79	1003	910	517	517
query80	575	447	369	369
query81	546	329	277	277
query82	830	142	113	113
query83	324	209	208	208
query84	334	125	103	103
query85	924	417	376	376
query86	328	239	232	232
query87	2014	1966	1842	1842
query88	3746	2702	2702	2702
query89	352	282	242	242
query90	1906	176	170	170
query91	163	155	131	131
query92	95	83	88	83
query93	1046	1032	578	578
query94	456	324	273	273
query95	589	396	307	307
query96	669	499	235	235
query97	2439	2436	2315	2315
query98	166	156	149	149
query99	782	768	654	654
Total cold run time: 235211 ms
Total hot run time: 151139 ms

[hello-stephen]

ClickBench: Total hot run time: 24.31 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 1322847012729a1e190651ebff44add931369c47, data reload: false

query1	0.01	0.00	0.01
query2	0.11	0.05	0.04
query3	0.26	0.09	0.08
query4	1.61	0.10	0.11
query5	0.25	0.25	0.24
query6	1.17	0.68	0.68
query7	0.05	0.01	0.01
query8	0.05	0.04	0.04
query9	0.42	0.34	0.36
query10	0.59	0.60	0.59
query11	0.19	0.15	0.14
query12	0.18	0.15	0.15
query13	0.50	0.50	0.50
query14	1.04	1.03	1.00
query15	0.64	0.62	0.62
query16	0.37	0.35	0.36
query17	1.15	1.13	1.13
query18	0.22	0.19	0.20
query19	2.08	1.95	2.00
query20	0.01	0.01	0.02
query21	15.48	0.18	0.14
query22	4.97	0.05	0.06
query23	16.26	0.33	0.12
query24	2.85	0.62	0.41
query25	0.13	0.07	0.05
query26	0.78	0.22	0.15
query27	0.04	0.03	0.04
query28	4.00	0.80	0.38
query29	12.51	4.13	3.28
query30	0.28	0.14	0.10
query31	2.77	0.56	0.32
query32	3.23	0.59	0.50
query33	3.23	3.18	3.13
query34	15.44	4.04	3.38
query35	3.37	3.35	3.32
query36	0.54	0.44	0.47
query37	0.10	0.07	0.06
query38	0.05	0.04	0.03
query39	0.04	0.02	0.02
query40	0.17	0.15	0.14
query41	0.09	0.04	0.04
query42	0.04	0.03	0.03
query43	0.04	0.03	0.04
Total cold run time: 97.31 s
Total hot run time: 24.31 s

[hello-stephen]

FE Regression Coverage Report

Increment line coverage 6.28% (234/3729) 🎉
Increment coverage report
Complete coverage report

[englefly]

run buildall

[englefly]

/review

[github-actions]

Reviewed all 116 changed paths at head 5a2df43e76166bd3fed18aed527a1b748fcb3594 using the supplied aggregate diff. Five new findings: four P2 issues affecting outer-join identity, null-extension, learned-feedback association and persisted replacement revisions; one P3 issue affecting live cache configuration. These can misapply optimizer estimates or lose persisted statistics; no transaction-data visibility defect was established.

Critical checkpoint conclusions:

• Goal and proof: the structural-key, injection, feedback and persistence pipeline is present, but the inline counterexamples show it is not fully correct. Supplied tests were inspected statically.
• Scope and reuse: the broad planner/profile/command changes serve this feature and reuse existing providers/internal SQL; no unrelated source change identified.
• Concurrency and locking: FE-local SET/DELETE/load serialize consistently and internal planner reads avoid lock reentry. Cross-FE timestamp generations fail to protect a replacement (P2 inline).
• Lifecycle: daemon retry/enable/stop behavior, memo/cache ownership and postprocessor rebuild order checked. Parent/fragment/runtime-filter registration follows rebuilding; no separate lifecycle defect confirmed.
• Configuration: mutable ratios and persistence flag are read at use; the pinned bound is explicitly startup-only. The new relation-table cache misses the mutable size/expiry callback (P3 inline).
• Compatibility: structural keys replace legacy fingerprints; internal-table column checks reject an incompatible schema. Mixed-version/restart behavior was not executed.
• Parallel paths: logical/physical plans, both literal modes, cloud/non-cloud access, partial/storage aggregation exclusions and the separate skew consumer checked. Outer self-join and nullable-side definition paths have the inline identity defects.
• Conditional/error paths: type/scope/value/STRUCT/age validation, unavailable internal storage and documented unknown-baseline behavior checked. No additional unsupported-shape or error-propagation issue confirmed.
• Test coverage: focused unit, golden and positive/negative regression coverage is substantial; the five reported cases and true distributed replacement races lack adequate coverage. No builds or tests were run under the static-only review contract.
• Expected results: persistence output and generated golden expectations were checked against code. The captured cluster corpus is historical syntax/shape evidence, not execution against this head.
• Observability: EXPLAIN/SHOW expose keys, modes, baseline/guard/application states; load/persistence/snapshot failures include useful logging. No additional critical metrics gap identified.
• Persistence and failover: FE-local authority, one-time loading and best-effort SQL are explicit. They do not protect a newer cross-FE persisted row from timestamp-bounded deletion (P2 inline); restart/failover was not runtime-validated.
• Writes and atomicity: no transaction-row, MoW or version-visibility mechanism changed. Local cache/write ordering is serialized; the persisted-statistics replacement race remains.
• FE/BE variables: no new transmitted fields or function ABI requiring corresponding BE changes identified.
• Performance and memory: scan caps, cached descriptions, empty-cache/relation guards and administrative cache bounds checked. Retained relation-key supersets were treated as a documented optimization tradeoff without a substantiated material impact. No runtime performance claim is made.
• Other concerns/user focus: no additional user focus was supplied; all initial risks and candidates have explicit evidence-based dispositions in the review ledger.

Existing review context: the mark-join flag and different-table inner-expansion bindings are fixed in this head. Six existing P2 concerns about expression metadata, set mappings, learned freshness/populations, skew freshness and learned-wildcard precedence remain known context and were not reposted. No existing P0/P1 comment requires an ID in existing_blocking_comment_ids.

Coverage limits: static analysis only; no builds, tests, cluster runs or performance measurements. All changed paths and assigned risk questions were covered, with no unresolved review candidate remaining at delivery.

Convergence: true after 3 rounds. All three final-round reviewers returned NO_NEW_VALUABLE_FINDINGS on the identical five-comment set; required coverage and candidate/risk dispositions are complete.

Review converged after 3 round(s).

[github-actions]

[P2] Keep each equality operand bound to its outer-join input

LEFT_OUTER_JOIN(a.k=b.v, Scan(t a), Scan(t b)) and the same join on a.v=b.k, selecting all four columns, get identical expansion keys here. For t(k,v)={(1,1),(2,1)}, they produce three versus two rows. Both operands become the original-table names t.k/t.v, the equality sorts them, and sideOperands sees the same {t.k,t.v} on both sides. A factor of 1.5 injected for the first therefore estimates three rows for the second as well, with no binding/freshness guard. Preserve the input/occurrence of each operand before canonicalizing an outer self-join, and cover this reversed-column pair. This is distinct from the existing different-table INNER JOIN thread: both inputs here read the same table.

[englefly]

Fixed in the branch ([fix](fe) Order the operands of an outer join expansion by their side):

• The operands of an expansion pair are now ordered by the side they are read from: the operand which belongs to join.child(0) is written first (the side is resolved by the slot exprId against the child outputs, so it is the same while a group is estimated with GroupPlan children and while the chosen plan is printed), and the pair is not sorted for a join whose sides are not interchangeable.
• Consequences: t a LEFT OUTER JOIN t b ON a.k = b.v and the same join on a.v = b.k now render t.k = t.v and t.v = t.k (different keys), while the same pair written in either order (b.v = a.k against a.k = b.v) normalizes to one key.
• An inner join keeps its operands sorted: the optimizer may swap its children and the count of an inner self join does not depend on which occurrence is called the left one.
• Tests: HboJoinConditionsTest pins the reversed outer self join, the either-order pair and the inner self join (6 cases), and org.apache.doris.nereids.stats.* + the HBO command suite pass (201 cases). The expansion regression suite has not been re-run against a FE rebuilt from this commit.

[englefly]

对不能交换顺序的join, join等式的左右需要和join孩子的左右保持一致

[github-actions]

[P2] Preserve null-extension in inherited column definitions

Consider Aggregate(gb=x) -> LEFT_OUTER_JOIN(t.k=u.k) -> [Scan(t), Project(u.k,coalesce(u.v,0) AS x)->Scan(u)], versus computing that same x in a project above the join. With t.k={1,2} and u={(k=1,v=0)}, the first has groups {0,NULL} (two rows); the second has {0} (one row). This merge forwards the right-side x definition unchanged across null-extension, so applyColumnDefinitions rewrites both grouping keys to the same Coalesce expression. Transparent projects then give both plans identical keys in both literal modes, and identical scan baselines pass freshness, allowing the other plan's pinned/learned group count to apply. Retain the nullable-side evaluation boundary when propagating definitions (or reject unsupported definitions), and add the paired plans.

[englefly]

null-extend 标签, 同时在null-extend 节点上添加编号

[github-actions]

[P2] Keep each learned output count with the inputs that produced it

UnionAll(Filter(k<10, Scan(t)), Filter(k<1000, Scan(t))) over the same unaliased key table containing k=0..999 can publish two scans under the same S fingerprint. The filters are embedded into their scan operators. If the narrow scan is visited first, this retains its output=10, while InputTableStatisticsInfo.merge chooses the other scan's larger input=1000 together with its predicate and RelationId. On the next identical query, computeFilter's scan-key fallback matches that retained k<1000 predicate and freshness accepts 1000 against the table's 1000 rows, then applies the unrelated output=10 to the 1000-row branch. Keep output and corresponding inputs together, using separate recent-run entries for different predicate/input signatures. Unlike the existing ScanRows-population thread, this mismatched pair passes both matching and freshness.

[englefly]

learned 条目不在本pr处理. learned hbo 要删掉重新实现

[github-actions]

[P2] Use a write revision that distinguishes replacements across FEs

create_time comes from System.currentTimeMillis() before the FE-local lock, so it is not a unique generation across FEs. Two concurrent SETs for one fingerprint can capture the same millisecond; after B's INSERT replaces A's row, a conditional stale deletion on A still matches B's acknowledged replacement through this <= predicate. Clock skew can likewise give a later replacement a smaller timestamp, and tombstoneCovers will suppress that replacement during a pending load too. This violates the stated guarantee that a newer SET on another FE survives removal of the judged entry, even when every SQL operation succeeds. Persist a distinct revision/CAS token for the entry and target that revision when a known entry is removed, rather than ordering independent writes by local wall-clock time.

[englefly]

修改del的顺序, 先删除table中的行, 成功再删除cache中的行. 去掉tombstoneCovers 机制

[github-actions]

[P3] Rebuild the relation-table cache when its configuration changes

relationTableIdCache is constructed with hbo_plan_info_cache_num and expire_hbo_plan_info_cache_in_fe_second, but this callback never rebuilds it while updating every sibling cache. For example, starting with both limits disabled and then setting positive bounds through ADMIN leaves this new cache unbounded and non-expiring for subsequent filtered queries until FE restart; reducing a positive size/TTL likewise keeps the original limits. Both settings are mutable and invoke this callback. Rebuild the relation-to-table cache with the updated size/expiry and carry over its entries, as done for the adjacent scan-filter cache.

[englefly]

这是learned hbo 专属的成员. learned hbo 后续pr要完全重做, 采用pinned 的处理方式替代 relatoinId的作用. 不在这个pr里实现

[hello-stephen]

TPC-H: Total hot run time: 28272 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 5a2df43e76166bd3fed18aed527a1b748fcb3594, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17622	5132	5022	5022
q2	2100	313	271	271
q3	10243	1362	809	809
q4	4679	457	335	335
q5	7445	793	535	535
q6	181	180	155	155
q7	745	799	596	596
q8	9271	1341	1015	1015
q9	5637	4204	4177	4177
q10	6842	1289	1031	1031
q11	424	243	224	224
q12	637	402	275	275
q13	18101	3683	3003	3003
q14	277	276	241	241
q15	q16	755	738	676	676
q17	1198	755	582	582
q18	7086	6180	6151	6151
q19	1114	940	582	582
q20	383	331	229	229
q21	5238	2103	2304	2103
q22	351	296	260	260
Total cold run time: 100329 ms
Total hot run time: 28272 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5632	5603	5590	5590
q2	687	565	552	552
q3	5232	5373	4923	4923
q4	2002	2134	1574	1574
q5	5237	5160	5663	5160
q6	265	195	153	153
q7	2328	2080	1922	1922
q8	3064	2929	2707	2707
q9	8201	8051	8182	8051
q10	3731	3672	3264	3264
q11	605	451	411	411
q12	683	692	512	512
q13	3465	3906	3236	3236
q14	306	314	298	298
q15	q16	679	705	644	644
q17	8356	7531	7334	7334
q18	12974	12135	12950	12135
q19	904	766	761	761
q20	2137	2144	1923	1923
q21	5556	4596	4773	4596
q22	503	437	404	404
Total cold run time: 72547 ms
Total hot run time: 66150 ms

[hello-stephen]

TPC-DS: Total hot run time: 150859 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 5a2df43e76166bd3fed18aed527a1b748fcb3594, data reload: false

query5	4304	579	449	449
query6	424	202	185	185
query7	4785	447	223	223
query8	332	168	165	165
query9	8769	3060	3093	3060
query10	478	312	256	256
query11	5972	3849	3566	3566
query12	150	93	85	85
query13	1247	438	313	313
query14	6105	3923	3678	3678
query14_1	3433	3384	3414	3384
query15	208	206	182	182
query16	948	465	453	453
query17	875	655	517	517
query18	2413	419	297	297
query19	195	162	124	124
query20	83	84	84	84
query21	205	127	105	105
query22	12911	12948	12808	12808
query23	13429	12932	12547	12547
query23_1	12594	12643	12597	12597
query24	7366	819	450	450
query24_1	459	456	468	456
query25	509	398	329	329
query26	1249	249	131	131
query27	2802	428	272	272
query28	4622	1915	1908	1908
query29	1538	583	462	462
query30	315	217	181	181
query31	1002	929	808	808
query32	142	85	90	85
query33	512	295	240	240
query34	885	849	487	487
query35	728	755	630	630
query36	818	811	709	709
query37	116	97	78	78
query38	1771	1725	1689	1689
query39	789	791	743	743
query39_1	755	737	743	737
query40	224	119	99	99
query41	73	69	70	69
query42	105	95	87	87
query43	336	367	307	307
query44	1290	712	676	676
query45	177	169	162	162
query46	835	954	560	560
query47	3048	2886	2894	2886
query48	293	309	222	222
query49	571	416	298	298
query50	672	270	207	207
query51	10531	10256	10485	10256
query52	78	85	68	68
query53	184	203	148	148
query54	240	208	201	201
query55	72	69	62	62
query56	228	222	221	221
query57	1662	1614	1542	1542
query58	281	258	256	256
query59	2110	2169	1987	1987
query60	279	243	223	223
query61	153	150	146	146
query62	380	335	286	286
query63	194	154	155	154
query64	2729	930	790	790
query65	3295	3248	3237	3237
query66	1782	429	316	316
query67	20596	20134	20084	20084
query68	3030	993	612	612
query69	392	290	267	267
query70	841	760	762	760
query71	299	234	222	222
query72	3056	2733	2410	2410
query73	527	522	297	297
query74	4632	4533	4333	4333
query75	2386	2287	1902	1902
query76	2332	1017	589	589
query77	353	444	262	262
query78	9924	9670	9135	9135
query79	998	837	510	510
query80	879	439	345	345
query81	608	320	266	266
query82	572	131	105	105
query83	305	191	187	187
query84	326	129	106	106
query85	888	436	391	391
query86	350	239	230	230
query87	1993	1911	1801	1801
query88	3561	2646	2627	2627
query89	334	273	239	239
query90	1803	164	161	161
query91	161	148	123	123
query92	96	87	90	87
query93	1013	952	572	572
query94	534	289	263	263
query95	569	384	304	304
query96	663	482	228	228
query97	2461	2434	2353	2353
query98	154	147	146	146
query99	797	745	640	640
Total cold run time: 235261 ms
Total hot run time: 150859 ms

[hello-stephen]

ClickBench: Total hot run time: 24.23 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 5a2df43e76166bd3fed18aed527a1b748fcb3594, data reload: false

query1	0.00	0.00	0.01
query2	0.09	0.05	0.05
query3	0.26	0.09	0.08
query4	1.61	0.11	0.12
query5	0.23	0.25	0.24
query6	1.18	0.67	0.68
query7	0.04	0.01	0.01
query8	0.05	0.04	0.03
query9	0.39	0.34	0.36
query10	0.57	0.58	0.57
query11	0.19	0.15	0.14
query12	0.18	0.15	0.15
query13	0.50	0.50	0.50
query14	1.02	1.02	1.01
query15	0.62	0.61	0.61
query16	0.36	0.34	0.34
query17	1.12	1.10	1.13
query18	0.20	0.20	0.19
query19	2.16	1.99	1.99
query20	0.02	0.01	0.01
query21	15.47	0.18	0.12
query22	4.95	0.05	0.05
query23	16.29	0.33	0.14
query24	3.02	0.67	0.32
query25	0.11	0.08	0.04
query26	1.39	0.22	0.16
query27	0.04	0.05	0.05
query28	2.94	0.73	0.36
query29	12.48	4.06	3.25
query30	0.28	0.13	0.10
query31	2.77	0.54	0.32
query32	3.23	0.60	0.50
query33	3.18	3.20	3.19
query34	15.34	3.97	3.38
query35	3.34	3.34	3.34
query36	0.56	0.45	0.41
query37	0.10	0.06	0.06
query38	0.05	0.04	0.04
query39	0.03	0.02	0.03
query40	0.17	0.14	0.14
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.04
Total cold run time: 96.69 s
Total hot run time: 24.23 s

[hello-stephen]

FE UT Coverage Report

Increment line coverage 50.65% (1714/3384) 🎉
Increment coverage report
Complete coverage report

[hello-stephen]

FE Regression Coverage Report

Increment line coverage 6.91% (234/3384) 🎉
Increment coverage report
Complete coverage report

[github-actions]

Static review of PR #69322 at e3ddd11. Four new P2 findings are recorded inline: an ignored join-expansion factor with unknown column statistics, an obsolete regression assertion, ambiguous null-extension paths, and a separate MAP-literal identity collision. No new P0/P1 finding was established, and the supplied threads contain no P0/P1 finding to carry forward. Existing issues were treated as duplicate fences.

Critical checkpoint conclusions:

1. Goal and proof: the change connects structural HBO keys, manual statistics management, learned feedback and diagnostics. Static tracing establishes the four defects; inspected tests do not establish executed success.
2. Scope and reuse: the broad change is focused on HBO and shares descriptor, identity, storage and guard helpers. No separate scope defect was substantiated.
3. Concurrency and locks: planner-local group state is single-threaded; admin and loader writes serialize under pinnedLoadLock while planning reads remain lock-free. Internal SQL can delay other admin operations. The existing cross-FE replacement/deletion race remains known context.
4. Lifecycle: one loader per FE retries until its one-time load succeeds; persistence-off leaves it available for a later enable. Memo descriptions and planning-time fingerprint snapshots have the expected query/cache lifetimes. No C++ static-initialization change applies.
5. Configuration: freshness/filter ratios and persistence are read dynamically; pinned-cache size is startup-only, and the scan limit is a session setting. The already-reported relation-table cache callback omission is not reposted.
6. Compatibility: lookup and publication consistently use the new FE keys. Internal-table missing-column detection rejects incompatible schemas with operator guidance. No new thrift field or BE row-storage format is introduced; rolling upgrades were not executed.
7. Parallel paths: filter/join/aggregate, logical/physical scans, staged aggregates, join types, broadcast costing and post-process group propagation were inspected. The unknown-statistics join branch misses the new multiplier.
8. Conditional checks: freshness, FILTER_SMALL, eligibility, scan limits and outer-join bounds have concrete purposes. The null-boundary path representation fails when a child index has multiple digits.
9. Test coverage: unit, golden and regression coverage exercises ordinary identity, injection, drift, persistence and negative input cases. Multi-digit paths and MAP literal collisions lack paired cases; restart, multi-FE and persistence-fault execution are not covered by this review.
10. Expected results: the new persistence output is consistent with its SQL under static inspection. The expansion regression has a deterministic assertion incompatible with its new equality-only key.
11. Observability: EXPLAIN/SHOW expose keys, baselines and application state; load/write failures have logs. The ignored-factor path nevertheless reports the expansion as applied. The existing swallowed stale-delete error remains known context.
12. Persistence and failover: internal SQL supplies ordinary transaction/journal behavior, with no custom EditLog record. Same-FE load/SET/DELETE ordering is serialized; the known cross-FE delete/replacement concern is not reposted. No failover simulation was performed.
13. Writes and atomicity: local SET/DELETE changes the cache after successful persistent SQL and retains it on explicit failure. No user-table write or delete-bitmap path changes here.
14. FE/BE propagation: no new wire variable is added; the scan limit uses FE forwarding metadata and EXPLAIN display stays local. Existing UDF metadata accessors cover all six Java/Python wrappers.
15. Performance and memory: empty-cache guards, relation prefilters, memoized descriptions and scan-count limits avoid unnecessary work. No additional concrete performance defect was established; allocation/runtime costs were not measured.
16. Other correctness: the two new identity collisions can apply unrelated pinned counts. Existing cast/order-field, ARRAY serialization, aggregate-producer, null-marker and learned freshness/matching/skew issues remain duplicate context. UDAF DISTINCT and direct VARIANT expansion hypotheses were dismissed after upstream legality/binding checks.

There was no additional user-provided focus. All changed paths and tracked candidates/risk items have dispositions. This review is static only: no builds, tests, cluster restart or failover execution was performed.

Review converged after 3 round(s).

[github-actions]

[P2] Apply the expansion to the unknown-statistics fallback

For INNER(t.k=u.k, Scan(t,1000 rows), Scan(u,1000 rows)) with unknown equality-column statistics, estimateInnerJoin returns at lines 265-273 before reaching this multiplier. This occurs with missing column statistics, or explicitly with enable_stats=false while HBO remains enabled. Injecting a factor of 100 therefore still estimates 1000 rows instead of 100000, although the Cartesian cap is 1000000; applyPinnedJoinExpansion also reports the factor as applied. Route the fallback estimate through the expansion tail and cover unknown-column inputs as well as analyzed tables.

[github-actions]

[P2] Update the regression for equality-only expansion keys

The query above has the equality t1.a=t2.a and residual t1.b>t2.c+1, but HboJoinConditions.keyParts now visits only hash conjuncts. Its canonical cannot contain lit(1:), so this assertion fails whenever the preceding setup succeeds, before the final SET/SHOW checks. testAResidualDoesNotChangeTheExpansionKey already asserts the new contract. Replace this expectation with equality-key equivalence across residual changes and verify that residual selectivity is applied after the expansion.

[github-actions]

[P2] Delimit child indexes in null-extension boundary paths

Concatenating decimal indexes conflates [1,0] with [10]. A reduced common input is:

Agg(group by x OR group by y)
  CROSS(F(f), LEFT(LEFT(a,P(u,x)),p), s0,...,s7,
        TopN(LEFT(b,P(u,y))))

Here P(u,z) projects u.k, coalesce(u.v,0) AS z; each LEFT matches keys, and the eight s inputs are scans. The sorted leaf prefixes F,J,S,...,S,T put the two branches at indexes 1 and 10. x gets path 0 through the enclosing LEFT's left child, then 10 at CROSS; y keeps its empty path through TopN, then also gets 10. Both grouping expressions become NullExtended<10>(Coalesce(col(u.v),lit(0:INT))). With a.k={1,2}, b.k={2}, u={(1,0)} and singleton dummy inputs, x has groups {0,NULL} and y only {NULL}. The common child and scan baselines are identical, so both literal modes collide and a pin can apply the other count. This uses 14 scans, below the default limit. Encode paths as delimited integers or a structured list. This is distinct from cache-order instability: the paths are stable but ambiguous.

[github-actions]

[P2] Encode MAP literal keys and values separately

MapLiteral reaches this fallback, whose getValue() is a map of literals. StringLikeLiteral.toString() quotes values without escaping: the one-entry MAP<STRING,STRING> values (key=a'='b, value=c) and (key=a, value=b'='c) both serialize as {'a'='b'='c'}. Encoding that complete text afterward preserves the collision. Consequently Filter(map_contains_key(mapLiteral,t.k))->Scan(t) has the same WITH_LITERAL fingerprint for both maps, while STRING k='a' matches only the second; identical scans pass freshness and an exact pin applies the unrelated count. Typed map literals are reachable through visitMapLiteral and a constant cast to MAP<STRING,STRING>. Recursively encode each key/value and its boundaries. This is separate from the existing ARRAY-element thread: maps never enter that special branch, so fixing its loop leaves this fallback collision.

[englefly]

/review

[github-actions]

Static review of #69322 at 6a5632d. Four distinct new findings: three P2 issues in mapping identity, residual expansion and pending-load visibility, plus one P3 metadata-lifetime issue. The feature is wired end to end, but these defects need correction.

Existing inline threads were treated as duplicate fences. Still-applicable concerns include ARRAY/MAP literal encoding, cast/order metadata, aggregate producer identity, ambiguous null-boundary paths, learned input/freshness/skew handling, unknown-statistics expansion, the stale residual regression assertion, cross-FE deletion and stale-cleanup error reporting. Mark-join identity, outer/equality side orientation and the former cache-order boundary-ID issue have targeted fixes. No existing P0/P1 inline finding was present to carry forward. No additional user-provided focus was supplied.

Critical checkpoint conclusions:

1. Goal and proof: structural identities, pinned management, learned feedback and diagnostics are present; the new findings and known threads prevent a clean correctness assessment. Tests exist but do not cover the four new triggers.
2. Scope and clarity: changes are cohesive FE HBO work; independently maintained expression encoders diverge in the mapping finding.
3. Concurrency and locks: optimization is single-threaded; query snapshots use concurrent caches and RuntimeProfile child traversal is locked. Commands and loader share an FE-local monitor, with SQL under it; no inverse lock cycle was demonstrated. That lock does not guarantee backend publication.
4. Lifecycle: memo snapshots, partial-stage exclusions and one-shot loader ownership were traced. Pending-load resurrection and historical relation-key retention are defects; no C++ initialization change applies.
5. Configuration: filter/drift ratios and persistence toggle are read on use; max-scans is session-scoped/forwarded; pinned capacity is startup-only. The already-reported mutable learned-cache callback omission remains.
6. Compatibility: no new FE/BE wire symbol or plan format; all six UDF implementations supply the added API. Structural keys replace old learned identities. Internal-table schema checks fail explicitly on incompatible columns; mixed-version/restart behavior was inspected statically.
7. Parallel paths: logical/physical plans, exact/no-literal lookup, partial/storage aggregations, legacy/MOW deletion and the separate cost consumer were checked. The mapping encoder is a missed path; cost freshness remains an existing thread.
8. Conditional guards: scan limits, unsupported shapes, small-filter gating and freshness fallbacks were traced. The new identity and residual findings use supported, known-stat paths.
9. Test coverage: unit/golden/regression coverage includes ordinary injection, validation, growth, pruning, caps, identity and counters; mapped DISTINCT, NULL-sensitive residuals, publication/load races and index retirement need coverage. No builds or tests were run, as required.
10. Changed results: the persistence .out matches its suite's static SELECT shapes/values; the known residual-key assertion contradicts the new equality-only contract. Golden fixtures were inspected with their generators; generation provenance was not independently established.
11. Observability: EXPLAIN use/skip/expansion annotations, SHOW baselines and SQL/load diagnostics support investigation. Stale-cleanup client error suppression is already reported.
12. Persistence/EditLog/failover: manual entries use internal-table SQL and existing transactions, with no new FE edit-log payload. COMMITTED and VISIBLE are distinct; the pending-load finding violates the required deletion lifecycle.
13. Data writes/atomicity: user-table read/write, partition versions and delete-bitmap algorithms are unchanged. Internal metadata deletion/load can restore a removed override; cross-FE replacement deletion is an existing issue. Crash behavior was not executed.
14. FE/BE variables: existing profile input_rows is reused; no new wire field. FE-only max-scans forwarding is present.
15. Performance and memory: cache-empty/relation/scan-count gates and memoized descriptions reduce hot-path work; the unbounded historical relation index defeats the pinned metadata bound.
16. Other concerns and limits: all changed-file families and risk items received a static disposition; no additional focus or unresolved candidate remains. Runtime execution, builds, failover experiments and result-generation provenance were outside the authorized static review.

Review converged after 2 round(s).

[github-actions]

[P2] Preserve DISTINCT in set-operation mapping keys

With agg_phase=3, compare SELECT COUNT(v) AS x FROM t GROUP BY k UNION SELECT x FROM u with COUNT(DISTINCT v) in its first branch (u.x is BIGINT). BuildAggForUnion produces the reduced tree:

Aggregate(gb=x)
  UnionAll
    Aggregate(gb=k, Count(v) AS x) -> Scan(t)
    Scan(u), mapping u.x

The DISTINCT variant keeps this logical shape, but this encoder writes only Count and its child, omitting isDistinct(). The child aggregate key contains only k, so both actual fingerprints collide in both modes. For t={(1,1),(1,1)} and u.x={1}, the UNION has two versus one rows; identical scan baselines accept the other query's pinned count. Encode the semantic fields already handled by expressionName in this binary path and cover mapped DISTINCT aggregates. Unlike the existing cast/order text-normalization and aggregate-producer threads, the canonical text already distinguishes this pair; the separate mapping hash loses that distinction.

[github-actions]

[P2] Preserve residual selectivity when expanding equality rows

For the reduced plan INNER(hash=a.k=b.k, other=(a.v IS NULL OR b.v IS NULL), Scan(a), Scan(b)), let each analyzed input have 1000 rows, k NDV=1000/no NULLs and v NDV=500/NULLs=500. The cross-input OR remains a join residual. A factor of 100 changes the equality estimate from 1000 to 100000 here, but StatisticsBuilder retains each v's absolute null count and base-column identity. FilterEstimation.visitIsNull therefore still estimates 500 for each disjunct, and visitOr returns at most 1000, effectively discarding the registered expansion. Later normalization cannot restore the lost factor. Scale the dependent populations consistently before applying residuals, or preserve the unexpanded residual selectivity. This reaches the multiplier with known column statistics and is separate from the existing unknown-statistics early-return issue.

[github-actions]

[P2] Wait for deletion visibility before completing the pending load

execUpdate also succeeds when DELETE returns SQL OK with COMMITTED after its publish timeout (DeleteJob.commit; the MOW insert path does too by default). If this FE's one-time load is still pending after startup/retry, DELETE then invalidates the cache and acknowledges success, but the subsequent loadAll can still read the old visible row. ensurePinnedLoaded reinstalls it and marks the load complete, so the deleted override stays active even after publication removes the stored row. Wait for VISIBLE or preserve deletion state across that pending load; the FE-local lock alone does not establish SQL visibility. This needs neither another FE/replacement nor a caught SQL error, unlike the existing persistence threads.

[github-actions]

[P3] Retire relation keys when pinned entries leave the cache

The pinned cache is bounded by hbo_pinned_stats_cache_num (5000 by default), but this set only grows: successful DELETE, stale cleanup and Caffeine eviction never remove relation keys. Repeated SET/DELETE for distinct table combinations therefore retains one full table-list string per historical combination for the lifetime of the FE, even when the pinned cache is empty. Load likewise registers rows beyond the cache's eventual admission bound. Bound/rebuild this index or track references from live entries so the configured cache bound also bounds its retained metadata. This is separate from the existing learned relation-table cache's hot-configuration issue.

[englefly]

/review

[github-actions]

Static review of #69322 at d54197a. Two new P2 fingerprint findings are reported inline: computed project outputs lose their bounded producer, and equivalent slot equalities receive different structural hashes. Existing threads and replies were checked; known issues are not reposted. No existing P0/P1 finding applies, so existing_blocking_comment_ids is empty. There was no additional user-provided focus.

Critical checkpoint conclusions:

• Goal and correctness: structural HBO keys, manual controls, freshness and collection are wired end to end; the two inline issues still break correct matching/reuse. Tests exist but do not cover these paired plans.
• Scope: broad, related FE feature work; physical-state preservation and legacy test moves support the same change. No unrelated implementation found.
• Concurrency: planner-local memo state and concurrent caches separate reads from the FE-local persistence/load monitor. SQL runs under that monitor; no new lock inversion established. Cross-FE stale replacement and delayed DELETE visibility remain existing threads 4336321664/4356183531.
• Lifecycle: per-query descriptions end with the memo; mutable holders are copied independently; startup loader retries and stops after successful loading. No cross-TU static initialization applies.
• Configuration: ratios and persistence are read dynamically; pinned size is explicitly startup-only; max-scans is session-forwarded. The relation-table cache hot-update omission remains existing 4336321670.
• Compatibility: no new FE/BE protocol or user-table storage format. Structural keys change; internal table schema checks fail visibly on missing columns and require operator recreation rather than automatic migration.
• Parallel paths: logical/physical readers, exact/agnostic and pinned/learned lookup, cloud startup, local SHOW and cost consumers checked. Skew costing freshness remains existing 4322187125.
• Conditional guards: FILTER_SMALL, stale fallthrough, unsupported scans/partial stages and numeric validation examined. Unknown-stat expansion and residual NULL selectivity remain existing 4355881604/4356183525.
• Test coverage: unit/golden and injection/drift/persistence regressions inspected. Real restart loading, cross-FE mutation, delayed publication and actual profile-feedback reuse lack execution evidence in this static review.
• Test results: supplied persistence output agrees with its SELECT fields/values on inspection; generation and pass status were not verified. The historical cluster corpus does not recompute current keys, and synthetic window-alias fixtures do not establish valid analyzed output bindings; the existing equality-only expansion assertion issue remains 4355881610.
• Observability: EXPLAIN/SHOW expose fingerprints, baseline, use/skip state; store/loader failures log context. Stale-cleanup failure acknowledgment remains existing 4354711309. No separate missing-metrics defect established.
• Persistence and transactions: normal internal SQL owns durability; memory mutations follow successful writes; one-time loading merges with putIfAbsent. Existing visibility/replacement threads limit restart/failover guarantees.
• Data writes and crashes: only HBO internal-table writes are added. Failed direct persistence preserves local entries and reports failure; no user-row visibility, delete-bitmap or BE memory-accounting code changes. Crash/reload behavior was not executed.
• FE/BE variables: no new transported field/operator; runtime feedback reads existing counters and planning snapshots use real PlanNodeIds. Learned row-population/strict-version and observation-mixing concerns remain existing threads 4322187106/4322187116/4336321656.
• Performance and memory: empty-cache/relations gates and cached summaries bound routine key work; pinned relation-index retention is existing 4356183536. No additional proven performance defect beyond reported matching issues.
• Other issues and limits: known aggregate-producer, NULL-path, MAP/cast/order and learned-wildcard threads were treated as duplicate fences. appendExpressionKey now recursively encodes ARRAY elements, but TopN still hashes rendered header text and remains covered by existing ARRAY thread 4354711292; builtin DISTINCT mapping flags are encoded in structural expression keys. No source coverage gap or unresolved candidate remains. No builds, unit tests, regression runs or source modifications were performed.

Review converged after 3 round(s).

[github-actions]

[P2] Keep computed project outputs bound to their producer

Consider this reduced supported plan over t(v)={0,1,1}:

Aggregate(group by a.x)  // compare grouping by b.x
  Cross
    Project(v+1 AS a.x) -> TopN(v ASC, 2)  -> Scan(t)
    Project(v+1 AS b.x) -> TopN(v DESC, 2) -> Scan(t)

Both definitions stored here become Add(col(t.v),lit(1)). resolveNodeExpressions therefore gives either parent the same grouping expression and fingerprint in both literal modes, although a.x has two distinct values and b.x only one. The common children and scan baselines match, allowing an exact pin for the two-group query to override the one-group query. Preserve the row-selecting producer when propagating project definitions and cover this paired tree. This is distinct from the existing aggregate-output producer thread: neither branch contains an aggregate, so fixing aggregateColumnDefinitions alone leaves this case.

[englefly]

扩展 nullextended的方法,使用child-index的方式标记计算expressoin的生成节点. 所有计算expr 都使用 bound([child-index], 表达式) 的方式记录

[github-actions]

[P2] Normalize commutative children before hashing

For Filter(t.a=t.b)->Scan(t) versus Filter(t.b=t.a)->Scan(t), the canonical renderer sorts the EqualTo operands, but this traversal writes their original order into the structural bytes. Upstream NormalizeBinaryPredicatesRule and SimplifyComparisonPredicate only commute constant/literal-left comparisons, so two distinct same-type slots retain that order. These equivalent filters consequently have different fingerprints in both modes despite identical printed structs and scan baselines, and a pinned count copied for one query misses the reversed query. Sort structural child keys for the commutative operators already recognized by isOrderInsensitive and add actual-fingerprint assertions for the reversed-column pair.

[englefly]

表达式使用结构化IR后, 仍然需要做文本IR时做过的排序工作. 补充结构化IR的排序

[englefly]

/review

[github-actions]

Static review of #69322 at 982056e (base snapshot 7a7e86c). Three new P2 findings are inline: temporary-partition identity collisions, missing storage-aggregate relations in the lookup summary, and lost sharing of volatile evaluations. These can misapply row-count overrides or make an advertised override inapplicable.

The 117 authoritative changed paths and their relevant upstream/downstream flows were covered by the main reviewer, two complete-review agents and an additional identity-risk reviewer. Existing threads and replies were duplicate fences. Current fixes for structural ARRAY boundaries, DISTINCT, mark joins and global equality-pair orientation were verified; remaining previously reported issues were not reposted. There are no existing P0/P1 inline comments to carry in existing_blocking_comment_ids. No additional user focus was supplied.

Critical-checkpoint conclusions:

1. Goal and proof: group-based identities, pinned/learned lookup, management and profile publication are connected, but the three inline cases violate intended identity/lookup behavior. Existing tests omit these cases.
2. Scope and reuse: this is a broad, coherent FE HBO feature; common key, state-copy and internal SQL facilities are reused. Mechanical physical-plan changes were reviewed with their callers.
3. Concurrency and locks: optimizer groups are query-local; asynchronous profile publication consumes captured keys. Loader and SET/DELETE share pinnedLoadLock and local stale removal compares entry identity under that lock. Internal SQL runs inside this dedicated lock. Previously reported publication-visibility and cross-FE replacement races still apply; no additional deadlock was established.
4. Lifecycle: group caches, physical-state copies and planning-time snapshots preserve ownership through postprocessing. The background loader retries a pending one-time per-FE load. Existing relation-index retention remains known. No C++ static-initialization change is involved.
5. Configuration: ratios and persistence are consulted live; the pinned-cache bound is explicitly startup-only. Session scan limits are forwarded. The existing relationTableIdCache hot-update omission remains known.
6. Compatibility: no new thrift layout or BE wire field is introduced. Removed fingerprint APIs have no remaining FE source/test callers. Structural keys intentionally replace old keys; the internal table checks required columns and rejects an older incompatible schema rather than migrating it.
7. Parallel paths: logical/physical plans, filters/joins/aggregates, pinned/learned/expansion consumers, file/internal scans, partial/final aggregation and cloud version access were inspected. The missing storage-wrapper summary is reported inline. Existing learned/skew freshness discrepancies remain duplicate-fenced.
8. Conditions and invariants: entry-type, literal-mode, finite-number, overflow, scan-limit, freshness and FILTER_SMALL checks were traced to concrete inputs. Equal partition counts do not prove whole-table selection, and volatile evaluation sharing must survive normalization, as the inline findings show.
9. Test coverage: all changed unit classes, 85 PLAN/51 TEXT golden cases, 21 historical captured cases, 11 regression suite bodies and eight exact legacy renames were inspected. Supplied tests do not exercise restart/load failures, cross-FE publication races, or the three new findings. The local race unit test uses persistence disabled.
10. Test results: the new persistence .out agrees with its declared SET/DELETE operations. Golden output characterizes the implementation rather than independently proving semantics; historical cluster hashes are not recomputed as current structural keys. The already-reported equality-only expansion versus residual-literal regression assertion remains unresolved. No builds, unit tests, regression tests or live FE experiments were run, as required by the review contract; generated-result provenance was not independently established.
11. Observability: SHOW baseline/state/detail, last-use information, EXPLAIN used/skipped markers and identifier-bearing store/snapshot logs are present. Existing stale-cleanup failure suppression remains known; the storage-summary finding explains a displayed key that silently cannot apply.
12. Persistence and failover: pinned metadata uses internal OLAP SQL transaction/journal machinery rather than a separate EditLog format. Each FE keeps one local loaded snapshot. Previously reported deletion visibility, replacement and failure-reporting defects still apply; runtime failover was not exercised.
13. Data writes and atomicity: direct SET/DELETE perform persistent work before cache mutation and report SQL failures. Stale cleanup's suppressed failure is already reported. The PR changes optimizer metadata and estimates, with no direct change to committed-row visibility, BE version bounds or MoW delete-bitmap handling.
14. FE/BE variable passing: runtime publication reads existing counter fields; added identity, display and management state stays in FE. No new BE parameter requires parallel send-path updates.
15. Performance and memory: group memoization, empty-cache/relation prefilters, scan caps and bounded caches were checked. The existing unretired relation-key index and missed learned-cache reconfiguration remain known. Full persisted-table startup loading is documented; no additional material performance defect was substantiated.
16. Other risks and limits: every candidate and initial risk has an explicit accepted, duplicate or dismissed disposition in the ledger. Suspected raw VARIANT expansion collisions were dismissed because raw VARIANT equality joins are rejected and cast operands have no expansion key. Remaining validation limits are static-only review, absent runtime race/failover proof and historical captured-plan evidence; no unresolved new candidate is being asserted as a finding.

Review converged after 2 round(s).

[github-actions]

[P2] Keep temporary partition IDs in the scan key

Consider Aggregate(gb=t.v) -> Scan(t TEMPORARY PARTITION(tp1)) versus the same tree reading tp2, with one ordinary partition p and two temporary partitions. Let p have two rows, tp1.v={0,1} and tp2.v={0,0}. of() passes the ordinary-only getPartitionNames().size(), so both one-partition selections take this branch and lose their IDs. Both literal modes then have identical fingerprints despite two versus one groups. Temporary IDs also miss getRowCountForPartitionIndex and receive the same estimated ordinary-table baseline, so freshness accepts the other population's exact pin. Retain selected IDs unless their set actually equals the ordinary partition-ID set, or reject unsupported temporary scans, and cover this paired plan.

[englefly]

temporary partition 上查询是非常少见的操作. 不收录hbo信息

[github-actions]

[P2] Count storage-aggregate scans in the relation summary

For an indexed DUP/MOW table, the default count-on-index implementation can produce Aggregate -> Filter(predicate) -> PhysicalStorageLayerAggregate(COUNT_ON_MATCH, embedded Scan(t)). pushdownCountOnIndex creates a new filter memo group, and buildNode includes the embedded scan in its full struct. This summary recognizes neither the storage wrapper nor its embedded relation, however, so that filter's relation key is empty. Copying its advertised EXPLAIN key into HBO SET registers relation t; applyPinnedStats then rejects the empty summary key before fingerprint lookup, so the pin never applies. Include the embedded storage relation consistently with buildNode and add a filter-over-storage-summary/lookup case.

[englefly]

用PhysicalStorageLayerAggregate内部的olapScan 代替它计算hbo

[englefly]

已按“用内嵌 relation 代替 wrapper 计算 HBO 信息”的方式修复（commit 见最新 push）：

1. describeExpression 对 PhysicalStorageLayerAggregate 直接描述它内嵌的 PhysicalOlapScan/PhysicalFileScan（getRelation()），canonical 现在是 S{...} 而不是 A{sl,op=...}(S{...})；
2. summaryOf 增加对应分支：scanCount = 1 且把内嵌 relation 记入 relations，使摘要与 buildNode/collectScans 一致（这正是空摘要导致 pinned 预过滤误杀的根因）；
3. wrapper 自身仍然不参与发布/广告（hasNoHboLookupPath 保持不变），可应用的键是它上层节点的键。

验证：org.apache.doris.nereids.stats.* + golden corpus + HBO 命令套件全绿。补充说明：真正构造 PhysicalStorageLayerAggregate 的单测还需要 physical scan 的测试脚手架（PlanConstructor 目前没有），这条用例留作后续补充。

[github-actions]

[P2] Preserve sharing of materialized volatile evaluations

Compare the supported reduced trees Filter(strcmp(x,x)=0) -> Project(uuid() AS x) -> Scan(t) and Filter(strcmp(uuid(),uuid())=0) -> Scan(t). The first compares one stored value with itself and keeps every row; the second compares independent random UUIDs and ordinarily keeps none. This definition substitutes the same UUID expression for both x references, the transparent project disappears, and appendExpressionKey encodes each leaf only as Uuid, losing the VolatileIdentity distinction between shared and independent calls. Both fingerprints therefore coincide in both modes, and identical scan baselines allow an exact pin to cross between them. The normal pushdown rule deliberately preserves volatile aliases. Preserve a stable evaluation-sharing binding, or reject these descriptors; this is distinct from the existing row-selecting-producer thread and needs no TopN/Limit/aggregate/join.

[englefly]

/review

[github-actions]

Static review of PR #69322 at b569fd2: two new P2 findings: aggregate-combinator identity and empty-key VARIANT subpath identity in HBO keys. Existing threads and replies were revalidated as duplicate fences; no existing P0/P1 finding applies. The commutative-child and ordinary aggregate-DISTINCT encoding fixes are recognized. Still-applicable descriptor, learned-statistics, persistence, cache-index and regression concerns remain in their original discussions and are not reposted.

Critical checkpoint conclusions:

1. Goal and proof: the change implements structural HBO keys, manual statistics, freshness checks and persistence. Unit, golden and regression coverage exercises the main flow; the inline combinator and empty-key VARIANT cases are missing and prevent complete key correctness.
2. Scope and reuse: this is a coherent but broad FE feature. Shared descriptor/store helpers serve its consumers; physical state-copy changes support key recovery after postprocessing. Eight legacy suites move without content changes.
3. Concurrency and locks: optimizer groups stay on the planner thread. Admin commands and the loader share pinnedLoadLock; persistence/load SQL runs inside that local monitor, while query lookup reads caches. Profile publication uses provider caches. No new catalog-lock ordering issue was found; the existing SQL-visibility and cross-FE replacement concerns remain.
4. Lifecycle/static initialization: group descriptions live with the memo; node-key snapshots survive through expiring provider maps. The startup loader retries failed loads and stops after success or internal-schema disable. There is no C++ static initialization change. Historical relation-index retention is already reported.
5. Configuration: row/filter thresholds and persistence switch are read dynamically; pinned cache size is explicitly startup-only. Session scan limits and EXPLAIN display are honored. The existing learned relation-cache hot-rebuild omission remains.
6. Compatibility/rolling versions: structural keys replace legacy keys; the new private internal table verifies its schema and rejects older incompatible layouts. No BE wire format or function ABI changes. Mixed-version key continuity is not established by runtime validation.
7. Parallel paths: logical/physical plans, literal modes, pinned/learned lookup, profile publication, storage wrappers, supported file scans, join types and Java/Python UDF identities were inspected. Existing path-specific issues remain fenced.
8. Conditional checks: unsupported/partial/storage paths, scan-count gates, small-filter guard, missing-input checks and freshness states have explicit fallback/skip behavior; no additional distinct conditional defect was verified.
9. Test coverage: positive/negative key, scan, freshness, command, profile-counter, input-alignment and local replacement cases are present. Both new pairs need actual-fingerprint coverage. Restart, multi-FE and runtime behavior were inspected statically only.
10. Expected results: golden producer/assertion code, both corpora and the persistence .out were reviewed; captured historical cluster data is not current execution proof. The already-reported equality-only expansion regression assertion still conflicts with the implementation. Generated-result provenance was not independently executed.
11. Observability: EXPLAIN keys/use/skip/expansion annotations, SHOW state/baseline/last-use details and persistence/snapshot logs cover the new decisions. Stale-cleanup client error reporting remains an existing issue; no further observability finding was verified.
12. Persistence/failover: changes use ordinary internal-table SQL rather than new EditLog records. Startup reload and local replacement checks were traced; previously reported visibility and replacement limitations remain. Failover execution was not tested.
13. Data writes/atomicity: writes affect HBO metadata only. User transaction publication and delete-bitmap execution are unchanged; command cache updates follow SQL success, subject to the existing publication-visibility concern.
14. FE/BE variables: the new controls are FE-side; the scan-limit setting has forwarding metadata. No new thrift field or BE execution parameter requires propagation.
15. Performance: lazy group summaries, memoized descriptions, scan limits and empty-cache/relation gates bound common lookup work. Initial full-table loading and monitor-held SQL were inspected; historical relation-index growth is already reported. No new independent performance defect was proved.
16. Core invariants/error handling: committed/uncommitted data visibility and BE memory ownership are unchanged. Cloud visible-version access and error propagation were traced; known learned-freshness and stale-cleanup error limitations remain in existing threads.
17. Other issues/user focus: complete changed-file and upstream/downstream sweeps found only the two new inline issues beyond existing discussions. There was no additional user-provided focus. No unresolved candidate or unreviewed assigned path remains.

Validation limit: static review only, as required. No source changes, builds, Doris unit/regression runs or live concurrency experiments were performed. Convergence describes the review process, not resolution of the reported bugs.

Review converged after 3 round(s).

[github-actions]

[P2] Preserve the nested function in aggregate-combinator keys

Compare the supported reduced trees Filter(array_contains(x,0)) -> Aggregate(gb=k, min_foreach(a) AS x) -> Scan(t) and the same tree with max_foreach(a). For t(k,a)={(1,[0]),(1,[1])}, the first keeps one group (x=[0]) and the second keeps none (x=[1]). Both calls are ForEachCombinator with the same ARRAY child; their nested Min/Max function is stored outside children(). This fallback therefore gives both the same expression identity. aggregateColumnDefinitions substitutes that identity into the filter, so both actual fingerprints coincide in both literal modes and identical scan baselines let an exact pin apply the other query's row count. Encode the combinator's nested function identity/bound name and cover this paired filter. This is distinct from the existing Cast/OrderExpression and producer-binding issues: neither type nor producer differs here, and their fixes cannot distinguish these functions.

[englefly]

需要将expression中非child的部分写入expr的IR结构

[englefly]

Fixed in the latest commit: see the corresponding change in GroupStructInfo/HboJoinConditions (counted qualifier and sub path components, VARCHAR-folded literal type in the NO_LITERAL key, escaped operand components, and the for-each combinator's nested function name in its identity).

[github-actions]

[P2] Encode VARIANT subpaths as a counted sequence

With the default element-at-to-slot rewrite enabled, compare Filter(IsNull(v)) -> Scan(t) and Filter(IsNull(v[''])) -> Scan(t). Empty object keys are supported: VariantSubPathPruning accepts the empty string, and LogicalOlapScan.computeOutput builds the extracted slot with the same name/qualifier as v and subpath [""]. For t.v={SQL NULL, {"":1}, {"a":2}}, the first filter keeps one row and the second keeps two. This join encodes both the base path [] and [""] as the empty string; every other structural field is identical, so both actual fingerprints collide in both modes despite different displayed canonicals. Identical scan baselines then allow an exact pin for one filter to override the other's estimate. Write the path length and each component separately, and cover the base-column/empty-key pair with actual-fingerprint assertions.

[englefly]

需要加count和长度前缀 到subpath的编码中

[englefly]

Fixed in the latest commit: see the corresponding change in GroupStructInfo/HboJoinConditions (counted qualifier and sub path components, VARCHAR-folded literal type in the NO_LITERAL key, escaped operand components, and the for-each combinator's nested function name in its identity).

[englefly]

/review

[github-actions]

Static review of #69322 at 1f1e778. Five new P2 findings are attached inline: qualifier-boundary collisions, failed string parameterization, ambiguous expansion operands, diluted selected-population freshness, and lost self-join multiplicity. They affect HBO matching or estimates, not a demonstrated change to returned SQL rows.

Reviewed flow: HBO SQL -> manager/cache/internal-table persistence -> startup loader; memo statistics -> structural key -> pinned/learned guards -> estimates; selected physical plan -> node-ID key snapshot -> runtime profile -> learned publication.

Critical checkpoint conclusions:

• Goal and proof: structural identity, manual management and feedback are connected end to end, but the five cases show incomplete matching/freshness guarantees. Relevant unit, golden and regression tests were inspected; these cases are missing.
• Scope and reuse: the change is broad but stays within HBO/planner identity and its tests. Plan reset methods reuse the common mutable-state copy helper.
• Concurrency and locks: optimizer/memo work is single-threaded; SQL sessions, background loading and profile publication are separate concurrent origins. The local pinned-load monitor serializes load/SET/DELETE and includes synchronous SQL, so management calls may wait on IO. Caffeine/concurrent containers and immutable entry fields protect shared reads; no additional lock-order cycle was established.
• Lifecycle: query memo descriptors and propagated group identity feed immutable planning-time fingerprint snapshots. The loader retries until a successful one-time load and avoids starting a second live copy. No C++ static-initialization change applies.
• Configuration: filter/drift ratios and session controls are read dynamically; pinned cache size is explicitly startup-only. The learned relation-table cache's hot-config omission is already reported.
• Compatibility: legacy fingerprint identities change; old internal schemas are detected and rejected rather than migrated. Restart/rolling-schema behavior was inspected statically only. No new FE-BE wire field is introduced.
• Parallel paths: checked logical/physical plans, postprocessing copies, join/aggregate/filter lookups, expansion costing, cloud/shared-nothing loading and internal/external scan treatment. The existing CostModel freshness issue is not reposted.
• Conditional guards: unsupported selectors/temporary partitions and scan-count limits are explicit. Missing fingerprint/input snapshots prevent unsupported wildcard publication; known learned guard defects remain existing-thread context.
• Test coverage: unit/golden cases cover key composition, identity, guards and exclusions; regression cases cover injection, drift, display and basic persistence. Restart, race, cloud execution and the new witnesses were not executed under the static-only contract.
• Changed results: the persistence .out baseline matches its stated deterministic SET/DELETE cases by inspection. The captured cluster corpus is historical and checks formatting/folding, not current runtime keys. The already-reported residual-expansion regression mismatch remains separate.
• Observability: EXPLAIN/SHOW expose keys, selected modes and guard decisions; persistence failures are logged. Stale-cleanup failure acknowledgement is already reported.
• Persistence/failover: internal SQL provides storage durability; no new EditLog record is added. FE-local serialization does not establish cross-FE replacement ordering or deletion visibility; existing threads already cover these cases.
• Writes and crash behavior: cache publication follows successful persistence calls. Existing publication/deletion visibility issues prevent a stronger atomicity claim; no user-data transaction/storage-write change is introduced.
• FE/BE variables: new controls act in FE planning; no additional transport path was required.
• Performance and memory: scan gates, memoization and cache bounds were reviewed. Relation-index growth is already reported; no additional concrete performance regression was established.
• Other invariants: committed-row visibility, delete bitmaps and BE allocator ownership are outside the changed execution/storage paths. The new findings concern FE estimate correctness.

Existing review threads and all raw replies were used as duplicate fences, including outdated anchors omitted from the thread markdown. Confirmed existing issues were not reposted; no existing P0/P1 comment applies, so existing_blocking_comment_ids is empty. User focus: no additional focus was supplied. Coverage includes all 117 changed paths and each initial risk focus. Validation is static only: no builds, tests, source changes or runtime SQL were performed.

Review converged after 2 round(s).

[github-actions]

[P2] Encode qualifier components separately in the slot key

With enable_nested_namespace=true, consider the supported reduced trees Aggregate(gb=z.v) -> CROSS(FileScan(cat.[x.y].t AS z), FileScan(cat.x.u AS [y.z])) and the same tree grouping by [y.z].v (brackets denote quoted identifiers). For t.v={0,1} and u.v={0,0}, they have two versus one groups. LogicalSubQueryAlias.computeOutputInternal gives those slots qualifiers [cat,x.y,z] and [cat,x,y.z]; alias-to-project preserves them. This join writes both as cat.x.y.z, with the same slot name/type, so their actual fingerprints coincide in both modes even though the displayed canonicals differ. The common child and relation summaries are identical, and unversioned file scans permit pins with unknown freshness, allowing the other aggregate's count to apply. Encode the qualifier count and each component separately. This is independent of the existing VARIANT-subpath thread: neither slot has a subpath.

[englefly]

Fixed in the latest commit: see the corresponding change in GroupStructInfo/HboJoinConditions (counted qualifier and sub path components, VARCHAR-folded literal type in the NO_LITERAL key, escaped operand components, and the for-each combinator's nested function name in its identity).

[github-actions]

[P2] Remove value-derived string lengths from NO_LITERAL keys

Compare Filter(Like(t.s,'%x_%')) -> Scan(t) with the same tree using '%long_%'. visitStringLiteral produces VARCHAR(4) and VARCHAR(7) literals, and compatible string coercion preserves those lengths. Both printed NO_LITERAL structs contain Like(col(...s),lit(*)), but this unconditional type field makes their actual structural fingerprints differ. Consequently a NO_LITERAL pin for the first pattern silently misses the second despite identical scans; the advertised constant-agnostic string parameterization only works for patterns of the same length. Give literals a mode-aware key that removes value-derived length/type distinctions when folding them, and add actual-fingerprint assertions for different-length string parameters.

[englefly]

Fixed in the latest commit: see the corresponding change in GroupStructInfo/HboJoinConditions (counted qualifier and sub path components, VARCHAR-folded literal type in the NO_LITERAL key, escaped operand components, and the for-each combinator's nested function name in its identity).

[github-actions]

[P2] Encode expansion operands before assembling pair text

Raw quoted column names can forge the operand boundary before fingerprintOf length-prefixes the complete pair. For tables internal.db.l and internal.db.r, name left column A a),col(internal.db.r.b and right column B b),col(internal.db.r.z (supported with the default Unicode-name setting). Compare these reduced INNER joins:

common: l.a=r.c1, A=r.c2, l.c3=B, l.c4=r.z
extra:  A=r.z              -- versus l.a=B

Both extra pairs become exactly EqualTo(col(internal.db.l.a),col(internal.db.r.b),col(internal.db.r.z)). The four common pairs make the operand sets identical too, so all key fields and actual fingerprints collide. In positional columns (a,A,c3,c4) / (c1,c2,B,z), give both tables rows (0,0,0,0),(0,1,1,1),(1,0,1,0),(1,1,0,1): the joins return four versus two rows despite identical marginal column statistics. A pinned expansion therefore crosses different equality populations without another guard. The supported setting disable_nereids_rules='INFER_PREDICATES,CONSTANT_PROPAGATION,REORDER_JOIN,REWRITE_JOIN_EXPRESSION' preserves these five direct hash predicates; duplicate elimination removes none. Encode comparison kind and each operand separately, then render the readable text. This differs from the existing table-binding/self-join threads: both sides are correctly bound to different named tables here.

[englefly]

Fixed in the latest commit: see the corresponding change in GroupStructInfo/HboJoinConditions (counted qualifier and sub path components, VARCHAR-folded literal type in the NO_LITERAL key, escaped operand components, and the for-each combinator's nested function name in its identity).

[github-actions]

[P2] Check distinct selected scan populations before summing

For INNER(a.k=b.k, Scan(t PARTITION(p1) AS a), Scan(t PARTITION(p2) AS b)), use a DUP table with every k=0 and runtime filters disabled. Learn the join with p1=10 rows and p2=10000 rows, then grow only p1 to 1,000 and wait for measured partition reports. The same aliases/index/partition IDs keep the fingerprint and FULL_MATCH unchanged. This merge and ofLearnedRows compare only table totals, 10010 versus11000 (+9.89%), so the default 10% tolerance accepts the old 100,000-row output although the join now has 10,000,000 rows and one selected input grew 100x. Each runtime count accurately measures its full selected population; no missing/zero or scan-counter discrepancy is needed. This loses the per-selected-scan tolerance described by this class and used by pinned freshness. Align by table/index/partition selection and check each distinct population before aggregating; unrelated stable scans should not dilute its drift. This requires no repeated-key merge and is distinct from the existing population-counter and output/input-pairing threads.

[englefly]

learned hbo 要重做, 不在这个pr里修改

[github-actions]

[P2] Preserve within-node scan multiplicity when merging feedback

Use a 1000-row DUP table and UNION ALL(Join(Scan(t a),Scan(t b),a.k=b.k), Join(Scan(t a),Scan(t b),a.k=b.k)), with identical local aliases and runtime filters disabled. The two hash joins have different runtime IDs but the same fingerprint, so genPlanStatisticsMap merges their inputs here. Each input list correctly contains two 1000-row occurrences; identityOf omits the occurrence, and this loop collapses even the first list to one before adding the second. The published entry records 1,000 rows, while the live join descriptor contains both aliases and freshness sums 2,000, rejecting feedback from the next unchanged query as 100% growth. Ordinary matching still passes because it clones the historical one-input list. Preserve the multiplicity within each node when aligning repeated measurements, or retain one complete matching measurement. This differs from the existing output/input-pairing thread: both branches have identical predicates, equal outputs and accurate full-scan measurements; the loss is the two occurrences within either one of them.

[englefly]

learned hbo 要重做, 不在这个pr里修改

[englefly]

/review

[github-actions]

Static review of #69322 at 6cf266b. Three new P2 findings concern sampled physical scans, lost explicit tablet selectors, and incomplete learned input coverage. Existing threads were treated as duplicate fences; no existing P0/P1 finding requires a blocking-comment ID.

Critical checkpoint conclusions:

• Goal and end-to-end behavior: traced parsing and management, memo identity, pinned/learned lookup, costing, physical-plan snapshots, profile publication and EXPLAIN. The feature is wired through these stages, but the three inline findings leave population-validation gaps. Unit, golden and regression coverage exists; this static review does not establish runtime success.
• Scope and clarity: the change is a broad HBO feature. Physical-plan state preservation supports its memo/EXPLAIN integration; no unrelated source change was identified.
• Concurrency and locking: query-local memo construction is thread confined. Background loading, management statements, asynchronous feedback and hot-config replacement interact through concurrent caches and the FE-local load lock. No additional deadlock was substantiated. Existing cross-FE replacement and SQL deletion-visibility concerns remain covered by their threads.
• Lifecycle: descriptors live with the query; feedback uses planning-time fingerprint snapshots. The loader retries and exits after one successful load. Per-FE caches and one-time loading are intentional. Existing unretired relation-index metadata remains covered by its P3 thread.
• Configuration: ratio settings are read dynamically; persistence changes affect the documented local lifecycle; the pinned-cache size is explicitly startup-only. The existing relation-table-cache reconfiguration omission remains threaded.
• Compatibility: these changes introduce no FE-BE protocol fields. The new internal persistence table checks required columns and reports obsolete schemas. Rolling-upgrade execution was not validated.
• Parallel paths: logical/physical scans, embedded storage aggregates, pinned/learned consumers, exact/constant-agnostic modes, cloud version reads, and join expansion variants were inspected. The new findings identify gaps specifically in embedded physical scan handling.
• Special conditions and errors: unsupported shapes, temporary partitions, samples, scan limits and fallback paths were traced. Direct persistence failures surface to SET/DELETE callers. The existing stale-cleanup error-reporting concern is not repeated.
• Tests and negative coverage: inspected the changed unit/golden/regression tests and all eight unchanged legacy moves. The mixed ordinary/storage-scan feedback case and sampled/tablet-selected count-on-index paths are missing. Existing expression/producer identity, learned population/version/matching, and expansion defects remain in their original threads.
• Changed results: inspected the persistence result file against its query projections and the golden resources against their generators and assertions. Generation history and runtime results were not independently verified. The existing equality-only expansion regression assertion mismatch remains covered by its thread.
• Observability: EXPLAIN exposes fingerprints, structures and applied/skipped states; persistence and snapshot failures include diagnostic context. No additional observability issue was substantiated.
• Persistence, failover and writes: only optimizer overrides are written through the internal table; user-table transaction/visibility and delete-bitmap logic are not modified. Local replacement guards and startup reload were inspected, with the existing multi-FE/SQL-visibility limitations retained. Crash/failover behavior was not executed.
• FE/BE variable passing and memory: no new backend wire contract or C++ allocation/ownership change applies. The session scan bound is forwarded; EXPLAIN control remains local.
• Performance and reuse: empty-cache/relation prefilters, memoized descriptions and scan limits reduce unnecessary work. Wrong estimates from the inline findings can change plan selection. No additional independently triggered hot-path defect was confirmed.
• Additional concerns and user focus: no additional user focus was supplied. All 117 changed paths and the five initial risk areas received static coverage. Suspected UDAF DISTINCT and direct VARIANT-equality collisions were dismissed after checking upstream binding/coercion; general state-copy concerns were checked against actual postprocessor ordering.

No builds or tests were run and no repository source was modified, as required by this review's static-only contract. Convergence is evaluated separately from the review verdict; existing P2/P3 threads are not declarations that those defects are fixed.

Review converged after 3 round(s).

[github-actions]

[P2] Reject sampled physical scans in storage aggregate descriptions

COUNT_ON_INDEX can turn Aggregate(count(*)) -> Filter(p) -> Scan(t TABLESAMPLE(...)) on an indexed DUP/MOW table into a new Filter(p) -> PhysicalStorageLayerAggregate(COUNT_ON_MATCH, embedded PhysicalOlapScan(sample)) memo group. buildNode passes that embedded physical scan here, bypassing this logical-only guard. HboScanDescriptor.of then emits the unsampled scan identity and full-table baseline. A pin for ordinary Filter(p) -> Scan(t) consequently matches and passes freshness for the sampled query, although translation preserves the sample and OlapScanNode later selects sampled tablets. Reject sampled PhysicalOlapScan too, including embedded storage relations, and cover the count-on-index path.

[englefly]

Fixed: a table sample now refuses the description for every scan class, and the storage aggregate path goes through the same check (the physical scan it embeds is covered), plus a sampled PhysicalFileScan is rejected like a snapshot.

[github-actions]

[P2] Count embedded storage scans when validating feedback inputs

For Join(t.k=a.c, Scan(t), Aggregate(COUNT(*) AS c, StorageAggregate(COUNT, embedded Scan(u)))), default no-group aggregate pushdown holds u in getRelation() rather than children(). Both collectors above miss it, so this completeness check accepts feedback containing only t. With runtime filters disabled, learn when every t.k=100 and u has 100 rows; then append 100 rows only to u. The version-free join key still matches, matching checks only recorded t, and learned freshness accepts unchanged t, reusing |t| although the join now returns zero rows. This differs from the existing population/multiplicity threads: the second table is absent entirely. Traverse embedded relations and reject publication if any input cannot be recorded; include wrapped file scans in the external-input exclusion too.

[github-actions]

[P2] Preserve explicit tablet selectors in physical scan identities

For an indexed DUP/MOW table, compare Aggregate(COUNT(*)) -> Filter(p) -> Scan(t) with the same tree reading t TABLET(id) for a proper tablet subset. COUNT_ON_INDEX introduces a new filter group over an embedded PhysicalOlapScan. The converter preserves its selected tablets, but this physical branch always sets specifiedTabletIds empty. The new filter therefore loses #tb and matches the full-scan filter pin; freshness also passes because rowsOf counts the full selected partitions. Execution still restricts reads to the chosen tablets. This needs no TABLESAMPLE, so extending the sample guard does not fix it. Carry manual-tablet provenance into the physical descriptor, or decline an unsafe descriptor, and test the count-on-index TABLET/full-scan pair.

[englefly]

/review

[github-actions]

Static review of 24badc84680733d7de6cb8e6be9f191903286b86: five new P2 defects are detailed inline. Fixes are needed for learned occurrence alignment, admin load-error reporting, SQL-mode-safe persistence, projection-stable outer-join expansion keys, and catalog-read locking.

Critical checkpoint	Conclusion
Feature goal and correctness proof	Structural HBO identity, pinned management and learned reuse are implemented; the inline defects prevent an unqualified correctness conclusion. Unit and regression cases were inspected, not executed.
Scope and clarity	Large but coherent FE feature/refactor; physical rebuild edits support preservation of planner state.
Concurrency and lock scope/order	Planner memo caches stay on the planning thread. Admin requests and the background loader share pinnedLoadLock; internal SQL occurs under that lock. Same-FE stale removal checks replacement identity. Freshness admin reads omit the table lock needed against concurrent rollup DDL (inline); no additional lock cycle was established.
Lifecycle and initialization	Fingerprints are snapshotted before memo release; the loader retries and stops after loading. Admin access must distinguish pending/failed load from an empty cache (inline). No C++ static-initialization issue applies.
Configuration	New ratios are read dynamically; scan limits are session-scoped; pinned cache capacity is explicitly startup-only. Existing relation-table cache reconfiguration thread remains a duplicate fence.
Compatibility	Persisted fingerprints are opaque and the new table schema is checked. Historical keys/cluster fixtures do not prove compatibility with current structural hashes. No new FE-BE wire schema or function ABI is introduced.
Parallel paths	Logical/physical scans, embedded storage relations, final/partial aggregates, cost consumers and plan-copy paths were traced. Projection-sensitive outer expansion is new; already-reported physical selectors and learned-cost freshness are not reposted.
Special checks and errors	Unsupported/sample/temporary/selected external shapes fall back with diagnostics. Initial-load failure is incorrectly hidden from admin callers (inline); existing stale-delete write-error and expansion-fallback threads remain separate known issues.
Functional, negative and unit coverage	All 117 changed paths were covered across the reviewers; constructed plans, golden resources, parser/admin negatives, drift, injection and eight legacy renames were inspected. New scenarios described inline lack corresponding coverage.
Expected results	Golden builder/resource case catalogs align; the cluster corpus is historical. The already-reported equality-only expansion regression assertion remains known. No test result is claimed passing.
Observability	EXPLAIN includes fingerprint/type/mode/applied/skipped/expansion details and SHOW includes baseline/freshness. Failed initial loading currently gives misleading successful admin results (inline).
Persistence and failover	Pins write through to an internal UNIQUE KEY table and reload per FE. SQL-mode-dependent byte corruption is new (inline); prior deletion-visibility/cross-FE replacement threads were deduplicated. Distributed durability/failover was not exercised.
Data writes and atomicity	No committed user-data, transaction visibility or delete-bitmap execution path is changed. HBO metadata uses SQL transactions plus local locking; persistence failures and crash/reload behavior were reviewed statically.
FE-BE variables	Added settings and UDF identity accessors are FE-side; no missing new wire-field propagation path was identified.
Performance and memory	Scan-count and relation prefilters, bounded caches, group memoization and lazy rendering were inspected. The existing retained-relation-key issue remains known. No benchmark or native memory-accounting validation was performed.
Other issues and user focus	No additional user focus was supplied. Suspected VARIANT expansion and distinct-UDAF cases were dismissed using coercion/binder reachability; other repeated concerns were fenced by existing threads.

Existing markdown threads and all 75 raw comments/replies were read, including older threads absent from the markdown window. Still-applicable identity/producer, learned population/version/skew, storage/tablet and persistence issues are already reported and are not duplicated. There are no supplied existing P0/P1 findings to carry forward.

Coverage limit: static source/diff/test inspection only; no builds, test execution, SQL execution, cluster startup or source modifications. The new findings describe code-proven triggers, not observed runtime reproductions.

Review converged after 2 round(s).

[github-actions]

[P2] Require an occurrence match before borrowing scan predicates

Learn the full scan of a 1,000-row t(k=0..999) through SELECT t.k FROM (SELECT 1) d CROSS JOIN t, then plan Filter(k=0) -> Scan(t) as a single-table query. The dummy consumes a relation ID, so the historical t scan has ID 1 and the current scan ID 0. Their new scan fingerprints match, but this check accepts the missing historical ID. getFilterAdjustedInputTableStatistics then looks up predicates at ID 1, gets null and constructs an unfiltered input; FULL_MATCH accepts the old unfiltered measurement. computeFilter's scan-key fallback consequently applies output=1000 to the one-row filter, and freshness passes because the unchanged full scan population is 1000. Require a verified current occurrence binding, or reject this lookup. This uses one complete historical measurement, with no merge, missing storage input or data change, unlike the existing feedback threads.

[englefly]

learned hbo 将重做, 不在这个 pr 范围

[github-actions]

[P2] Surface initial-load failures to statistics admin commands

After an FE restart with persisted pins, make the initial internal-table SELECT fail, for example while its backend or cloud compute group is unavailable. ensurePinnedLoaded returns normally on loadAll()==null and throughout its retry backoff; this method then presents the empty/partial cache as the complete set. HBO SHOW reports no stored entries and HBO DELETE STALE STATISTICS acknowledges success without inspecting or deleting persisted stale entries. The daemon later loads those untouched entries. Admin callers need an explicit load-success result and an error while the required load is incomplete. This differs from the existing per-entry DELETE-failure thread: no DELETE is attempted here and its failed-entry counter stays zero.

[englefly]

Fixed: ensurePinnedLoaded now reports whether the required load completed; the admin entry point (getAllPinnedPlanStatistics) raises an error while it is incomplete instead of presenting a partial cache.

[github-actions]

[P2] Quote persisted structures for the internal session's SQL mode

With global sql_mode including NO_BACKSLASH_ESCAPES, pin an advertised filter on a supported quoted table named t\q. HboScanDescriptor leaves that backslash in the scan token, but StatisticsUtil.escapeSQL(struct) doubles it. execUpdate creates an internal session from the global defaults without normalizing SQL mode, so the INSERT stores two literal backslashes while SET acknowledges the correct one-backslash entry in memory. After restart, loading that row registers the doubled table name as its relation key; the identical live query uses the original name and is rejected by mayHavePinnedEntryForRelations before fingerprint lookup. Persist the structure without changing its bytes by using quoting consistent with the internal parsing session, or set an explicit compatible SQL mode for this write, and cover the persist/reload round trip.

[englefly]

Fixed: the internal session of execUpdate clears NO_BACKSLASH_ESCAPES, so the values written with escapeSQL are parsed the way they were produced and a structure with a backslash keeps its bytes across a restart.

[github-actions]

[P2] Keep outer-join expansion keys stable under column pruning

Compare LEFT_JOIN(t.k=u.k, Scan(t(k,v)), Scan(u(k,w))) for SELECT * with Project(t.k,u.k) -> LEFT_JOIN(t.k=u.k, Project(k)->Scan(t), Project(k)->Scan(u)) for a key-only selection. ColumnPruning inserts the narrower child projects and the project-elimination passes retain them. This loop therefore puts t.k,t.v|u.k,u.w in the first key's sides field and only t.k|u.k in the second. The join reads the same rows under the same equality condition, yet an expansion copied from the first EXPLAIN silently misses the second because applyPinnedJoinExpansion only looks up that fingerprint. Bind the equality operands to stable input occurrences without including unrelated output columns. This is distinct from the existing outer-self-join collision: the equality population is identical here.

[englefly]

Fixed: an outer join now orders each equality pair by side (left child operand first) and writes no output columns, so column pruning cannot change the key. A pair whose two operands come from one child, or name the same column, is refused (no key); the sides/sideOperands output-column scan is gone. Added paired tests for the side ordering, both refusal cases, and a key-only project below each child.

[github-actions]

[P2] Snapshot catalog metadata under the table read lock

Run HBO SHOW or DELETE STALE for a pinned table while another session drops a rollup/materialized index. Neither admin caller takes the table read lock before entering of(). This getIndexIdList() calls getIndexIdToMeta(false), which copies a filtered view of OlapTable's plain HashMap. MaterializedViewHandler.processDropMaterializedView concurrently removes from that map through deleteIndexInfo under the table write lock; rollup-job publication also inserts under that lock. The unlocked copy can therefore throw ConcurrentModificationException and abort the admin command. Capture the needed local index/partition metadata under the matching read lock, releasing it before the cloud visible-version RPC, and add a coordinated reader/DDL case.

[englefly]

Fixed: the index metadata of a pinned entry is now captured under the table read lock (released before the visible-version rpc), so a concurrent rollup/mv drop cannot abort HBO SHOW / DELETE STALE.

[englefly]

/review

[github-actions]

Reviewed exact head d299294 using the supplied 117-file aggregate patch. One new P2 finding: a from_unixtime predicate can reuse an exact HBO pin across different session time zones. The implementation therefore still needs correction; this is not an approval.

Existing threads were independently rechecked against this head and used as duplicate fences, including the older raw comment archive. Known concerns remain in expression/producer identity, inner equality incidence, learned input alignment/populations/skew freshness, cross-FE replacement/deletion visibility, stale-cleanup error reporting, and the partially addressed index/partition snapshot. They are not reposted. No existing P0/P1 comment applies, so existing_blocking_comment_ids is empty. The failed-initial-load and SQL-mode fixes, pruning-stable outer expansion keys, sampled/temporary scan refusals, counted qualifier/subpath encoding, DISTINCT naming and commutative-key sorting are present.

Critical-checkpoint conclusions:

1. Goal and correctness: the structural-key, pinned-statistics and admin/EXPLAIN flow is implemented and has targeted tests, but the new time-zone defect and existing concerns prevent full correctness. No storage-engine visibility or delete-bitmap behavior is changed.
2. Scope and reuse: this is a large, coherent HBO feature; it reuses internal SQL execution, session contexts and Caffeine caches. Physical rebuild changes support retaining memo identity.
3. Concurrency: independent admin/planner sessions, the loader daemon and profile workers share caches. Local monitors, concurrent containers, volatile state and the profile child lock cover their intended scopes; cross-FE deletion and incomplete metadata snapshots remain existing issues. Planning does not initiate persistence loads.
4. Lifecycle: group descriptions stay planner-thread scoped; publication snapshots fingerprints before memo release. Loader retry/start and context restoration were traced. Existing relation-index retention concerns remain.
5. Configuration: ratio/persistence settings are read dynamically; the pinned-cache size is documented as startup-only. The missing relation-table cache reconfiguration is already reported.
6. Compatibility: structural keys replace legacy keys, and incompatible internal-table schemas are explicitly rejected. No new wire format or FE-BE field is introduced; restart/mixed-FE behavior has the existing persistence limits.
7. Parallel paths: logical/physical, storage wrappers, filter/join/aggregate, exact/agnostic, expansion/learned/skew, and cloud/local version paths were inspected. Known feedback and physical TABLET gaps remain covered by existing threads.
8. Conditions and errors: sample, temporary-partition, delimiter and unsupported-shape refusals have concrete reasons. Startup-load failures now reach admin callers; stale-cleanup per-entry failure reporting remains an existing issue.
9. Test coverage: unit/golden/regression cases cover identities, counters, guards, local races, injection and admin SQL. They lack proof of real restart/multi-FE and coordinated metadata races; the new VARCHAR-valued time-zone case is absent.
10. Expected results: output fixtures and assertions were inspected. The equality-only expansion regression's residual-literal assertion is still covered by existing comment 4355881610. Test execution and generated-result provenance were not verified.
11. Observability: EXPLAIN keys/used/skipped states, SHOW details and failure logs expose the main decision paths; no additional distinct observability defect was established.
12. Persistence/failover: internal-table DDL/DML use existing journal/transaction machinery. One-time FE-local loads, shared-row replacement and publication visibility retain the already-reported limitations.
13. Writes/atomicity/crashes: local SET/DELETE/load phases are serialized and failed direct writes retain the active entry. This does not establish cross-FE compare-and-delete or visibility guarantees; those remain existing findings.
14. FE-BE variables: none added; runtime feedback reads existing counters. No BE ownership or protocol change requires separate memory-tracker validation.
15. Performance: memoized descriptions, lazy text, relation prefilters and scan-count limits bound ordinary planning work. Existing retained-index/cache-bound issues were deduplicated; no additional substantiated performance finding remains.
16. Other risks and user focus: the suspected VARIANT expansion collision is blocked by analyzer casts/origin checks; comma-table descriptors are explicitly refused. All investigated candidates have accepted, duplicate or evidence-backed dismissed dispositions. No extra user focus was supplied.

Validation and coverage limits: static inspection only, as required. No builds, tests, benchmarks, cluster execution or source changes were performed. These limits do not imply that the existing unresolved review threads are fixed. All verified new findings are included inline.

Review converged after 2 round(s).

[github-actions]

[P2] Include from_unixtime in the time-zone-sensitive key

For Filter(Like(FromUnixtime(t.ts),'1970-01-01 00:%')) -> Scan(t) with BIGINT ts=0..3599, the predicate keeps 3600 rows in +00:00 and zero in +08:00. FromUnixtime has a BIGINT-to-VARCHAR signature, so neither its arguments nor its result activates the date-type check in usesSessionTimeZone; its name is also absent from this list. The structural fingerprints are consequently identical across the two sessions. A WITH_LITERAL pin copied in UTC then overrides the other session's filter estimate, because its unchanged scan baseline passes freshness. Include from_unixtime in the session-dependent identity and cover actual fingerprints for the same nonconstant predicate in both zones.