Skip to content

[feature](HBO) Add history-based optimizer statistics with structural struct-info keys - #69322

Open
englefly wants to merge 25 commits into
apache:masterfrom
englefly:hbo-structinfo-squashed
Open

englefly wants to merge 25 commits into
apache:masterfrom
englefly:hbo-structinfo-squashed

Conversation

@englefly

@englefly englefly commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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
englefly marked this pull request as draft October 20, 2026 06:25
@englefly

Copy link
Copy Markdown
Contributor Author

run buildall

@englefly

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

// arguments keep a different number of rows in another time zone
return className + '<' + encodeSlotComponent(sessionTimeZone()) + '>';
}
return className;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
Map<String, String> tables = new TreeMap<>();
Plan joinPlan = (Plan) join;
List<Plan> relations = joinPlan.collectToList(planNode -> planNode instanceof CatalogRelation);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// identity names the sub tree only, not how often the plan references it
out.count(child.mapping.size());
for (Expression mapped : child.mapping) {
out.text(renderExpression(mapped, mode));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

String table = HboScanDescriptor.qualifiedNameOf(scan.getTable());
// the input rows of a scan node are the rows it read, i.e. the number a catalog row
// count (which also ignores the predicates) can be compared with
recordedRowsByTable.merge(table, input.getInputRows(), Long::sum);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

continue;
}
compared = true;
String tableState = recorded == live[0] ? STATE_LIVE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Optional<PlanNodeAndHash> planNodeAndHashOpt;
// the publish path keys a join with the constant agnostic form, and that is the
// form which the read side publishes for skew ratios as well
planNodeAndHashOpt = HboUtils.getHboPlanNodeAndHash(physicalHashJoin,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// wildcard that matches by fingerprint alone. A published entry always carries the rows of at
// least one scan (see InputTableStatisticsInfo#hasInputTables, the publish path drops entries
// which recorded none), so an empty input list can only be a manual injection
for (RecentRunsPlanStatisticsEntry injected : planStatistics.getRecentRunsStatistics()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

break;
case JOIN:
out.text(node.flattenable ? "inner" : String.valueOf(node.joinType));
out.text(distinctIdentityConditions(node.conditions));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

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

@hello-stephen

Copy link
Copy Markdown
Contributor
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

Copy link
Copy Markdown
Contributor
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

Copy link
Copy Markdown
Contributor
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

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

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

@englefly englefly changed the title [feature](fe) Add history-based optimizer statistics with structural struct-info keys [feature](HBO) Add history-based optimizer statistics with structural struct-info keys Oct 21, 2026
@englefly

Copy link
Copy Markdown
Contributor Author

run buildall

@englefly

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

if (!slot.getOriginalTable().isPresent() || !slot.getOriginalColumn().isPresent()) {
return Optional.empty();
}
return Optional.of(HboScanDescriptor.qualifiedNameOf(slot.getOriginalTable().get()) + "."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
Map<ExprId, Expression> definitions = new LinkedHashMap<>();
for (Child child : node.children) {
definitions.putAll(child.node.columnDefinitions);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// rows, and the drift comparison of the published entry expects the rows of every scan
PlanStatisticsWithInputInfo published = outputPlanStatisticsMap.get(publishedKey);
outputPlanStatisticsMap.put(publishedKey, new PlanStatisticsWithInputInfo(
published.getId(), published.getPlanStatistics(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

String sql = "DELETE FROM " + FULL_QUALIFIED + " WHERE `fingerprint` = '"
+ StatisticsUtil.escapeSQL(fingerprint) + "' AND `" + CREATE_TIME_COLUMN
+ "` <= '" + createTimeLiteral(generation) + "'";
StatisticsUtil.execUpdate(sql);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Cache<String, Map<RelationId, Set<Expression>>> scanToFilterCache =
buildHboScanToFilterCache(cacheNum, expireSeconds);
scanToFilterCache.putAll(provider.scanToFilterCache.asMap());
provider.scanToFilterCache = scanToFilterCache;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@hello-stephen

Copy link
Copy Markdown
Contributor
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

Copy link
Copy Markdown
Contributor
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

Copy link
Copy Markdown
Contributor
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

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

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

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

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

@englefly

Copy link
Copy Markdown
Contributor Author

run buildall

@englefly

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed PR #69322 at 25063e6 using the supplied merge-base diff. Two new P2 findings: cache-order-dependent null-extension fingerprints can defeat a copied pin, and INNER self-join expansion keys lose operand incidence across multiple equalities. Both are inline with concrete plan examples. No new P0/P1 finding; existing_blocking_comment_ids is empty because the supplied threads contain no P0/P1.

Critical checkpoint conclusions:

  1. Goal and proof: the structural-key, management, lookup, feedback, EXPLAIN and persistence routes are connected; the two new identity defects prevent the intended reuse guarantees. Unit and regression cases were inspected statically.
  2. Scope and clarity: all 116 changed paths were covered. The broad change serves HBO; eight legacy suites are pure renames. Physical-plan state-copy edits retain memo provenance.
  3. Concurrency and locking: admin sessions and the loader serialize stored/cache mutations with pinnedLoadLock; planner lookups do not acquire it. Internal SQL can hold that management lock during IO, but no concrete deadlock was established. Existing cross-FE replacement deletion remains reported in discussion_r4336321664.
  4. Lifecycle: startup loader retries and stops after its one-time load; local entries win over loaded rows. Cache bounds and expiry were traced. Memo warmup order is the new lifecycle defect in the first inline.
  5. Configuration: new live thresholds and persistence toggle are read on use; pinned-cache capacity is explicitly startup-only. The existing mutable relation-cache omission remains discussion_r4336321670.
  6. Compatibility: new persistence schema uses named columns and checks required columns; old schema/unavailable storage fails management operations and retries loading. No FE-BE wire schema or backend function symbol changed. Mixed-version execution was not run.
  7. Parallel paths: logical/physical filters, joins, aggregates, final versus partial/storage aggregation, costing, publication, commands, cloud/shared-nothing contexts and EXPLAIN were checked. Known learned/skew freshness concerns remain in their existing threads.
  8. Conditions and invariants: filter-small guard, expansion caps and preserved-side floors, finite/precision checks, selectors, unsupported inputs and literal modes were traced. The second inline identifies a missing multi-conjunct self-join identity invariant. Transaction visibility, BE memory ownership and delete-bitmap protocols are unchanged.
  9. Test coverage: focused JUnit and SQL cases cover normal and negative paths, but omit the two new scenarios; the race test covers only a sequential same-FE replacement. Actual restart, concurrent multi-FE and runtime behavior remain unvalidated under the static-only contract.
  10. Expected results: golden resources and the persistence .out were inspected; no additional definite mismatch was established. Generation and execution were not verified.
  11. Observability: EXPLAIN fingerprints, used/skipped states and expansion details plus SHOW baseline/state/detail expose matching; persistence/load logs include relevant identifiers. No separate actionable observability defect established.
  12. Persistence and failover: stored writes/deletes precede local cache mutation, and SQL errors propagate. Existing internal SQL/catalog mechanisms provide durability; one-time load provides restart recovery. Cross-FE judged-generation deletion is already reported, not reposted.
  13. Writes and crash atomicity: no user-table transaction protocol changed. Persist-before-cache SET and delete-before-cache invalidation leave recoverable durable outcomes on crash; FE-local cache/table state is not a cross-FE atomic update, as reflected in the existing deletion thread.
  14. FE-BE variables: runtime counters use existing profile names; the added settings govern FE planning. No new wire field requires additional send-path handling.
  15. Performance: empty-cache/relation gates, per-group memoization and scan limits reduce repeated key work. Those gates interact with boundary numbering as reported; no additional concrete performance regression established. Persistence IO stays off normal lookups.
  16. Final reconsideration: expression identity, nullable projections, mark joins, self joins, residuals, subpaths, shared memo children, input alignment, empty input, publication merging and alternate consumers were rechecked. Unreachable/synthetic candidates were dismissed with evidence; known issues were deduplicated.

Existing-thread disposition: the reported single-pair outer expansion and different-table binding examples now use side-aware/original-table bindings; the supplied mark-semi and coalesce examples receive markers. Related marked CROSS_JOIN/constant-definition omissions remain covered by the existing marking/null-extension threads. Existing expression metadata, folded set mappings, learned input/output pairing, input population/zero, strict versions, skew freshness, injected wildcard priority and relation-cache configuration concerns were not reposted. None is an existing P0/P1 finding.

User focus: no additional focus was supplied; the whole PR was reviewed. Static analysis only: no builds, tests, source changes or runtime validation. No outstanding concrete candidate or changed-path coverage gap remains within that scope.

Review convergence: reached after 2 review rounds. All three latest reviewers returned NO_NEW_VALUABLE_FINDINGS on the same frozen two-comment set; required static coverage and candidate/risk dispositions are complete.

Review converged after 2 round(s).

node.joinType = join.getJoinType();
node.markJoin = join.isMarkJoin();
if (join.getJoinType().isOuterJoin()) {
node.nullExtensionBoundaryId = ctx.nextBoundaryId++;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Make null-extension IDs independent of memo cache order

Consider Aggregate(gb=x) -> LEFT_JOIN(s, LEFT_JOIN(t, Project(u, coalesce(u.v,0) AS x))). With no cached HBO entries, EXPLAIN describes the joins bottom-up in separate Ctxs, so the grouping definition becomes NullExtended<0>(NullExtended<0>(...)). Pin only the printed three-table aggregate key, then explain the identical query with HBO enabled and an empty learned cache: applyPinnedStats skips the two-table child through its relation-key prefilter, so the first three-table lookup builds the outer boundary as 0 and its cold inner boundary as 1. The grouping definition is now NullExtended<0>(NullExtended<1>(...)), changing both fingerprints and preventing the copied pin from matching. Cached children return without rebasing their IDs. Assign stable boundary identities when composing cached subtrees and cover the EXPLAIN-to-pin transition; this is separate from the already-reported missing null-extension marker.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null-extension 使用 child-index 路径记录 null 生产者. 前面版本用 ctx 的 idGen 会产生不确定的编号

private static String canonicalFirst(Join join, Expression conjunct, String left, String right) {
Integer leftSide = sideOf(join, conjunct.child(0));
Integer rightSide = sideOf(join, conjunct.child(1));
if (isCommutative(join.getJoinType()) || leftSide == null || rightSide == null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve operand incidence across inner-join equality pairs

Sorting each pair independently loses which self-join occurrence supplies an operand. For t(k,v,w)={(1,1,1),(2,1,2)}, INNER_JOIN(a.k=b.v AND a.v=b.w, Scan(t a), Scan(t b)) returns one row, while the same join on a.v=b.k AND a.v=b.w returns two. Both keys contain the same sorted original-column pairs {t.k=t.v, t.v=t.w} and operand set, since INNER joins carry no side bindings. An expansion factor of 0.5 pinned for the first therefore estimates one row for the second too; this path has no structural/freshness check to separate them. Canonicalize one global side swap for the entire condition set, preserving shared-operand incidence, rather than swapping each equality separately. This is a distinct multi-conjunct INNER self-join case from the existing single-equality outer-join thread.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为同名表的 self join , inner join 也需要像 left join 那样调整 equal join 条件 的左右顺序.

@englefly

Copy link
Copy Markdown
Contributor Author

/review

### What problem does this solve?

Problem Summary: the key of an injected join expansion named the alias a query gave a relation
(`a.k = b.k`), so `t1 AS a JOIN t2 AS b` and `t3 AS a JOIN t4 AS b` shared one key although they read
other tables and expand differently; the attempt to add the alias to table bindings could not work,
because a group being estimated has `GroupPlan` children (no relation below them to inspect) and the
concrete plan qualifies the operands as `internal.db.a`, which the alias table did not index. The
expansion entry carries no data state to guard it, so the factor of one join was applied to the other.

An expansion key now requires every operand of the join conditions to be a direct slot of a catalog
table (`SlotReference.getOriginalTable()` and `getOriginalColumn()` present): the key names the column's
own table and name, so it is the same while a group is estimated and while the chosen plan is printed,
and it does not depend on aliases or on the shape of a qualifier. A join whose operands are computed
columns (a cast, an aggregate output, a derived alias) has no expansion key at all: EXPLAIN prints no
`join type=join_expansion` line for it and no lookup matches it, instead of sharing a key with another
table pair. An outer join also keeps the columns of each side, which is what distinguishes the
preserved side; an inner join stays commutative.

### Release note

A manually pinned join expansion entry is now bound to the tables its conditions read; a join whose
operands are not direct table columns can no longer carry an expansion entry.

### Check List (For Author)

- Test: Unit Test
    - `HboJoinConditionsTest` pins the outer join side, the residual literals, the two joins over
      different tables with the same aliases, the computed operand without a key and the structural
      hash (5 cases pass); the regression suite which injects expansions reads its fingerprint and
      struct from the EXPLAIN annotation, and has not been re-run against a FE rebuilt from this commit.
- Behavior changed: Yes - expansion keys are bound to the tables of the conditions; computed operands
  are refused.
- Does this need documentation: No
### What problem does this solve?

Problem Summary: the identity of a join was its type and its conditions. A mark join, which implements a
non-nullable `IN` subquery as `Project(mark AS m) -> LEFT_SEMI_JOIN(mark=true, t.k = u.k)`, preserves
every row of its left side and produces a marker column, while the same semi join without the marker
drops the unmatched rows: with t = {1, 2} and u = {1} the first plan outputs two rows and the second
one, with the same join type, the same condition and the same input scans. Both plans rendered one key,
and since their inputs are identical the freshness comparison could not separate them either, so a
learned or pinned count measured on one was applied to the other.

The mark flag is now part of the join identity in both forms: the canonical string writes `J{<type>,mark,
c:[...]}` and the structural key writes the flag, so the two plans keep different keys in both literal
modes. Every join fingerprint moved once by this change (the flag is a field of the identity even when it
is false).

### Release note

A mark join and the plain semi join of the same columns no longer share an hbo statistics entry.

### Check List (For Author)

- Test: Unit Test
    - Two corpus cases pin the mark semi join and the plain one over the same scans; a test asserts their
      keys differ in both literal modes and that the canonical writes the flag. The regenerated file
      gained those cases and moved the fingerprints of the join cases (the intended change), and 200
      cases of the stats package plus the HBO command suite pass. The regression suites were not re-run
      against a FE rebuilt from this commit.
- Behavior changed: Yes - the mark-join flag is part of a join key.
- Does this need documentation: No
### What problem does this solve?

Problem Summary: the operand identity of a join expansion is the column of a table, so in a self join
both sides of an equality resolve to the same table: `t a LEFT OUTER JOIN t b ON a.k = b.v` and the same
join on `a.v = b.k` both rendered `t.k = t.v` (the pair was sorted for commutativity), and the side
information (`sides:[...]`) is the same for both sides of a self join. The two plans keep a different
number of rows, and an expansion factor pinned for one was applied to the other without any data state
to guard it.

The operands of a pair are now ordered by the side they are read from: the operand of the left child is
written first, so `a.k = b.v` and `a.v = b.k` keep different keys, while the same pair written in either
order (`b.v = a.k` against `a.k = b.v`) keeps one. An inner join stays commutative: the optimizer may
swap its children and its count does not depend on which occurrence is called the left one, so its
operands remain sorted by their identity.

### Release note

An hbo join expansion entry of an outer self join is no longer applied to the join of the same table with
the columns reversed.

### Check List (For Author)

- Test: Unit Test
    - `HboJoinConditionsTest` pins the outer self join with the columns reversed (different keys), the
      same pair written in either order (one key) and the inner self join (one key); 201 cases of the
      stats package plus the HBO command suite pass.
- Behavior changed: Yes - the operand order of an outer join expansion pair is the side it is read from.
- Does this need documentation: No
…oundary id

### What problem does this solve?

Problem Summary: the column definitions of a transparent project are inlined into the node above it, but a
side of an outer join is NULL for the rows the join does not match: a definition computed there which can
turn NULL into a non-NULL value (a coalesce, an `is null`, a udf, ...) denotes a different value above the
join. `Aggregate(gb = x)` over `LEFT_OUTER_JOIN(t.k = u.k)` with `coalesce(u.v, 0) AS x` computed on the
right side and the same expression computed above the join rendered one `A{gb:Coalesce(...)}` although the
first returns the groups `{0, NULL}` and the second `{0}`; a pinned or learned count of one applied to the
other on unchanged scans.

Every description now numbers the null extensions it contains in build order. A definition which crosses a
null-extended side is wrapped with the id of that boundary (`NullExtended<0>(Coalesce(...))`) when the
expression can turn NULL into a non-NULL value (the classification the outer join push down already uses,
extended over the sub tree, with a udf treated conservatively). The id is assigned inside one description,
which is memoized on the group, so the estimation path and the EXPLAIN path render the same marker. A
definition which cannot change its value (arithmetic, a cast, a plain column) is not wrapped, so the keys
of those plans do not move.

### Release note

A pinned or learned hbo statistics entry of a plan whose value is computed on the nullable side of an outer
join is no longer applied to the plan which computes it above the join.

### Check List (For Author)

- Test: Unit Test
    - Two corpus cases pin the pair (below the join against above it) and their keys differ in the marker;
      201 cases of the stats package plus the HBO command suite pass. The regression suites were not
      re-run against a FE rebuilt from this commit.
- Behavior changed: Yes - a null-extended definition is marked with the id of the boundary it crossed.
- Does this need documentation: No
…d report its failure

### What problem does this solve?

Problem Summary: a deletion removed the in-memory pinned entry first and the stored row second; when
the stored deletion failed the statement still succeeded, and the failure was compensated with a
tombstone that the lazy load replayed: the tombstone had to carry a generation (the create time of the
row it judged) to avoid deleting a row another FE wrote afterwards, and two FEs can capture the same
millisecond or disagree on the clock, so a newly acknowledged SET could be removed by the replayed
deletion.

The stored row is now removed first and the in-memory entry only afterwards: when the stored deletion
fails the statement fails (`AnalysisException`) and the entry stays active, so an entry the load reads
later always describes an entry the user was told is active. The tombstone bookkeeping, its generation
and the replay at load time are removed; the load installs the stored rows as they are, which makes the
whole cross-FE generation/revision question moot for the deletion path.

### Release note

`HBO DELETE STATISTICS` now fails when the persisted entry cannot be removed from the internal table,
and the entry stays active in that case.

### Check List (For Author)

- Test: Unit Test
    - `HboPinnedStatisticsRaceTest` keeps the case that a replacement is not removed and drops the
      generation cases; `HboShowStatisticsCommandTest` and the input-statistics suite pass.
- Behavior changed: Yes - a failed persisted removal now fails the statement instead of being replayed.
- Does this need documentation: No
…anup going entry by entry

### What problem does this solve?

Problem Summary: a pinned SET put the entry into memory and then wrote the stored row as a best-effort
step, so a failed write left an entry which only existed in that FE and disappeared on the next restart
without the statement saying so. A stale cleanup removed the entries one by one and the stored removal of
one of them could now abort the whole statement (its in-memory entry removed first in the previous
ordering, its row left behind).

A pinned SET writes the stored row first and, when that fails, fails the statement and leaves the memory
untouched - the same order the deletion takes, so memory and table cannot disagree about an entry the
user was told about. `HBO DELETE STALE STATISTICS` keeps going when one entry's stored row cannot be
removed: that entry stays active and is judged again by the next run, and the statement reports how many
entries it removed and how many it kept for that reason.

### Release note

`HBO SET STATISTICS` now fails when the entry cannot be persisted to the internal table, and
`HBO DELETE STALE STATISTICS` no longer aborts on one entry whose stored row cannot be removed.

### Check List (For Author)

- Test: Unit Test
    - `HboPinnedStatisticsRaceTest`, `HboShowStatisticsCommandTest` and the input-statistics suite pass
      (the persistence path is disabled in these tests, the statement-level behaviour is exercised by
      the hbo regression suites which run with the internal table enabled).
- Behavior changed: Yes - a failed persisted SET fails the statement; a stale cleanup skips such an entry.
- Does this need documentation: No
…build counter

### What problem does this solve?

Problem Summary: the boundary a definition crossed was numbered by a counter of the description being
built (`Ctx.nextBoundaryId++`). Each description is memoized per group and may be built as the root of its
own context, so the number a boundary got depended on which group was described first and on whether a
child description was already cached: describing the two-table join of a nested pair on its own numbered
it 0, and composing it later into the three-table description left it 0 while the outer boundary also got
0; when the prefilter of a pinned entry skipped the child, both boundaries were numbered inside one
context instead and became 0 and 1. The same plan therefore rendered `NullExtended<0>(NullExtended<0>(...))`
or `NullExtended<0>(NullExtended<1>(...))` depending on the cache, and a fingerprint copied from EXPLAIN no
longer matched.

A marker now carries the child indices from the boundary to the consumer instead of a number: the boundary
itself writes no index, and a node with more than one child prepends the index of the child the definition
came from. The code is a property of the assembled tree - a cached child description keeps the text of its
own subtree and the parent only prepends its index - so it is the same whatever order the descriptions were
built in, and two boundaries can no longer share a marker.

### Release note

The hbo statistics key of a plan whose value is computed below an outer join no longer depends on the order
the memo describes its groups in.

### Check List (For Author)

- Test: Unit Test
    - The regenerated corpus renders `NullExtended(...)` for the boundary of the pair case; the golden
      suite (27 cases) passes. The stats package is covered by the gate of the next commit.
- Behavior changed: Yes - the null-extension marker is a child-index code, not a boundary id.
- Does this need documentation: No
…n expansion key

### What problem does this solve?

Problem Summary: the operands of every equality were sorted on their own, which lost the incidence of a
self join: `INNER_JOIN(a.k = b.v AND a.v = b.w, Scan(t a), Scan(t b))` and the same join on
`a.v = b.k AND a.v = b.w` both rendered `{t.k = t.v, t.v = t.w}` with the operand set `{t.k, t.v, t.w}`,
although with t(k,v,w) = {(1,1,1),(2,1,2)} the first keeps one row and the second two: the first equates
the two columns of a with two columns of b, the second equates a.v with both of them. An expansion factor
pinned for one was applied to the other, and this store has no data state to guard it.

The pairs of an inner join are now ordered by one global orientation of its two sides: every operand of
the left child is written first, and the lexicographically smaller of the two possible orientations (all
pairs as given, or all pairs flipped) is the key. Flipping the whole set keeps the join commutative, while
ordering the pairs on their own lost which operands share a column. An outer join preserves a side, so its
orientation is fixed (the operand of the left child first), and a pair whose operands come from the same
side is ordered by identity.

### Release note

An hbo join expansion entry of an inner self join with several equality conditions is no longer applied to
the join with another incidence of the same columns.

### Check List (For Author)

- Test: Unit Test
    - `HboJoinConditionsTest` keeps its cases for the outer self join (the reversed columns differ), the
      either-order pair and the inner self join (one key); 200 cases of the stats package plus the HBO
      command suite pass. The multi-conjunct incidence case needs a three column scan fixture and is a
      follow-up test.
- Behavior changed: Yes - the operand order of an inner join expansion pair is one global orientation.
- Does this need documentation: No
### What problem does this solve?

Problem Summary: an expansion key named the whole condition set, residuals included, so the factor of
`a.k = b.v` could not be used by `a.k = b.v AND a.v > b.w`, and the two conditions had to be kept apart
by a residual text which itself was not oriented with the equality pairs: `INNER(a.k=b.v AND a.v>b.w)` and
`INNER(a.v=b.k AND a.v>b.w)` rendered the same key although the residual touches one of the equality
operands in the first plan only.

A key now describes the equality conditions only: the factor is the fan out of the equi join, and the non
equality conditions keep their own selectivity on top of it. `JoinEstimation` takes an expansion factor
that scales the equality estimate *before* the residual conditions are applied, and the estimator applies
the injected factor that way, so one entry serves every query with the same equality pair whatever
residual it adds, and a selective residual still lowers the estimate. A join whose equality operands are
not direct table columns still has no key.

### Release note

An hbo join expansion entry is matched by the equality conditions of a join and scales their estimate; a
residual predicate is applied on top of it. The factor of an existing entry is relative to the equality
estimate now, so entries tuned with the previous meaning should be reviewed.

### Check List (For Author)

- Test: Unit Test
    - `HboJoinConditionsTest` pins that a residual does not change the key; 200 cases of the stats package
      plus the HBO command suite pass. The regression suite which injects factors reads its numbers from
      the annotation and now asserts the reported base is at most the larger input; it has not been
      re-run against a FE rebuilt from this commit.
- Behavior changed: Yes - the key of a join expansion entry is the equality pair, and the factor scales
  the equality estimate instead of the larger input.
- Does this need documentation: Yes - the meaning of a JOIN_EXPANSION factor changed.
…ingerprint preimage

### What problem does this solve?

Problem Summary: the structural key of a group wrote the rendered text of its conditions, its branch
mappings and its header expressions as one length prefixed string. Within that string the values of an
expression were still concatenated by the printed grammar (`lit(` ... `,` ... `:type`), so a value which
carried those characters could stand for a field boundary of its own: the array `['a','b']` and the
singleton array whose element is `a:TEXT),lit(b` produced the same preimage, although they match
different rows.

The fingerprint preimage now serializes expressions structurally as well: every expression writes its
kind, its value defining fields (a literal's type and value, a slot's qualifier and name) and its
children, each with its own length, and the conditions are sorted and de-duplicated over those bytes.
A composite literal keeps one field per element, so an element cannot forge the boundary of another.
The display form (canonical, shape, EXPLAIN, the struct a user pastes) is unchanged; only the fingerprint
moves. A missing semantic field (a cast target, an order direction) is not addressed here and remains a
per-expression follow-up.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - The regenerated corpus changes only its fingerprint lines (the shapes, canonicals and scan tokens
      are byte identical); 200 cases of the stats package plus the HBO command suite pass.
- Behavior changed: Yes - all fingerprints moved once (the display form did not).
- Does this need documentation: No
### What problem does this solve?

Problem Summary: the previous commit introduced the length prefixed expression key but the condition
collections of a filter, an aggregation and a join still wrote the rendered text of their expressions
(and the join case was formatted with the new mark-join flag, so the replacement of that site did not
apply). The array literal case was therefore still one key for `['a','b']` and for the singleton array
whose element is `a:TEXT),lit(b`: the elements of a composite literal were only separated by the printed
grammar again.

The three sites now write the structural expression key as well, which also restores the identity
suffixes the display name carries (a udf's scope and id, a match analyzer, a search dsl, a lambda's
arguments, the distinct flag), the variant sub path of a slot (`v['age']` is a different column) and the
sorting/de-duplication of the conditions over their bytes. A corpus case pins the two arrays apart.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - The new corpus case `filter_with_an_injected_delimiter_array_literal` asserts the two arrays keep
      different fingerprints in both literal modes; 201 cases of the stats package plus the HBO command
      and pinned-race suites pass.
- Behavior changed: Yes - the fingerprints of the condition sites moved once (the display form did not).
- Does this need documentation: No
…it reads

### What problem does this solve?

Problem Summary: the definition of a computed column kept only the expression, not the sub tree which
produces the rows it reads, so two plans whose definitions share the expression rendered one key. With
t(v) = {0,1,1}:

    Aggregate(group by a.x)                Aggregate(group by b.x)
      Cross                                  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 become `Add(col(t.v),lit(1))` and both grouping keys render the same though `a.x` has
two values and `b.x` one: the Cross (which names both branches) is the same in both plans, so nothing in
the key says which branch the aggregation groups by. An exact pin of the two group query therefore
applied to the one group query.

A computed definition which reads the output of a node that selects the rows (a top-n or a limit) is now
bound to that producer: the marker records the child indices from the producer to the consumer, which is
the same structural code the null extension marker uses. `NullExtended` becomes the general `Bound`, with
the null extension as the case where the crossing is gated on the definition being able to turn NULL into
a non NULL value; a row selecting boundary binds every definition. The code, a property of the assembled
tree, is the same whether the child was described fresh or reused from the memo.

### Release note

An hbo statistics entry of a plan whose computed column is bound to a top-n or a limit is no longer
applied to the plan computing the same expression over another such producer.

### Check List (For Author)

- Test: Unit Test
    - The regenerated corpus renders `Bound(...)` for the null extension pair case; 201 cases of the stats
      package plus the HBO command suite pass. A corpus case for the top-n pair needs the top-n fixture and
      is a follow-up.
- Behavior changed: Yes - a computed column above a top-n or a limit is bound to that producer.
- Does this need documentation: No
…ural key

### What problem does this solve?

Problem Summary: the canonical renderer sorts the operands of a commutative expression, but the
structural key wrote the children of every expression in their written order. `NormalizeBinaryPredicatesRule`
and `SimplifyComparisonPredicate` only commute constant-left comparisons, so `Filter(t.a = t.b)` and
`Filter(t.b = t.a)` rendered one struct and hashed two: a pinned row count copied from one query was not
found by the query which only reverses the two columns.

The structural key now normalizes the operators the renderer already recognizes as order insensitive
(equality, null safe equality, and, or): each child is serialized on its own, the child keys are sorted by
their unsigned bytes, and the sorted keys are written length prefixed. The code does not read the rendered
text, so two operands whose text would tie still have a deterministic order. The corpus fingerprints of
plans whose written operand order differed from the sorted one move once.

### Release note

An hbo statistics entry of a plan with a commutative expression inside a filter or a join condition is no
longer sensitive to the written operand order.

### Check List (For Author)

- Test: Unit Test
    - A test asserts `Filter(t.a = t.b)` and `Filter(t.b = t.a)` share the fingerprint in both literal
      modes; the regenerated corpus (70 fingerprint lines) and 201 cases of the stats package plus the
      HBO command suite pass. The regression suites were not re-run against a FE rebuilt from this commit.
- Behavior changed: Yes - the structural key sorts the children of a commutative expression.
- Does this need documentation: No
### What problem does this solve?

Problem Summary: a temporary partition is a short lived copy which a manual job replaces, and the queries
which read one are rare, so a key recorded for it is not worth the risk of a stale match; the partition
identity was also not part of the scan key in every path, which made two scans of one table differ in
places but match in others.

An olap scan whose selected partitions contain a temporary partition now refuses the description with the
reason `temporary partition of <table>`, like a tablesample does: no struct info, no fingerprint and
therefore no pinned or learned entry is generated for that sub tree, and nothing it feeds into can be
applied. The refusal is reported by EXPLAIN and by the `HBO SHOW` verdicts.

### Release note

An hbo statistics entry is no longer injected for a query which reads a temporary partition.

### Check List (For Author)

- Test: Unit Test
    - 202 cases of the stats package and the HBO command suite pass. A case with a temporary partition
      needs the olap table fixture of the catalog tests and is a follow-up.
- Behavior changed: Yes - a scan of a temporary partition is refused instead of being given a key.
- Does this need documentation: No
### What problem does this solve?

Problem Summary: `PhysicalStorageLayerAggregate` (the wrapper a count-on-index plan puts over the scan it
reads) was described as a node kind of its own, so the canonical string of the filter above it named the
scan (`F{...}(A{sl,op=COUNT_ON_MATCH}(S{t}))`) while the structural summary of that filter recognized
neither the wrapper nor its embedded relation: the wrapper is a catalog relation without children, so
`summaryOf` fell through both of its leaf branches and produced an empty summary. The pinned entry
lookup uses the summary as its necessary condition, so a pin copied from the EXPLAIN of that filter
registered the relation and was then rejected by the prefilter before its fingerprint was compared - the
entry was stored and never applied.

The hbo information of the wrapper is now the one of the relation it embeds: the description of the
group is the description of that scan, and the summary counts it as one scan of that table. The wrapper
itself is still not published or advertised (its translator delegates to the embedded relation and the
read side applies statistics at filter/join/aggregation nodes only), but the filter above it now has a
summary which matches its canonical string, so its pin can be found.

### Release note

A pinned hbo statistics entry of a filter above a count-on-index plan is applied again.

### Check List (For Author)

- Test: Unit Test
    - The stats package, the golden corpus and the HBO command suite pass. A dedicated unit case which
      builds a `PhysicalStorageLayerAggregate` still needs the physical-scan harness (no
      `PlanConstructor` helper for it yet) and is left for a follow-up.
- Behavior changed: Yes - the canonical string of a group whose node is the storage wrapper is the scan
  it reads, and the fingerprints of those groups moved once.
- Does this need documentation: No
### What problem does this solve?

Problem Summary: five fields of the structural key were written in a form which cannot be read back, so
two structures the key is supposed to separate collided (or an equivalent one was split):

1. A slot's qualifier was joined with `.` (`[cat,x.y,z]` and `[cat,x,y.z]` both became `cat.x.y.z`, two
   groups against one for a grouping over such a slot).
2. A variant sub path was joined with NUL, so the base column (`[]`) and the empty key (`[""]`) both
   became the empty string (`IsNull(v)` keeps one row of t.v={SQL NULL,{"":1},{"a":2}}, `IsNull(v[''])`
   keeps two).
3. A string literal's type (VARCHAR(4) for '%x_%', VARCHAR(7) for '%long_%') was written even in the
   NO_LITERAL key, so the constant agnostic key of two patterns of different length still differed.
4. The operand of an expansion pair was written raw, so a quoted column name containing `),col(` could
   forge the boundary between two operands.
5. The nested function of `ForEachCombinator` is a field, not a child, so `min_foreach` and `max_foreach`
   had the same identity although they keep different groups.

A qualifier and a sub path are now counted sequences of length prefixed components; a string literal folds
its type to VARCHAR in the NO_LITERAL key; an operand escapes `%`, `,` and the parentheses before the pair
text is assembled; and a for each combinator writes the name of its nested function (`min_foreach`).

### Release note

Hbo statistics keys distinguish quoted identifier components, variant sub paths, string patterns of
different length and aggregate combinators of different nested functions.

### Check List (For Author)

- Test: Unit Test
    - The regenerated corpus (142 fingerprint lines) and 202 cases of the stats package plus the HBO
      command suite pass. The paired fingerprint fixtures for the qualifier and variant cases need file
      scan and variant slot fixtures and are follow-ups.
- Behavior changed: Yes - the five fields above are encoded without ambiguity.
- Does this need documentation: No
…ax_foreach

The two combinators are one class with a nested aggregate function field, so the key has to write the nested function name; the paired filter over the aggregate now asserts the fingerprints differ in both literal modes.
### What problem does this solve?

Problem Summary: the description refused a `LogicalOlapScan` with a table sample, but a storage layer
aggregate embeds a `PhysicalOlapScan`, and a sampled physical scan passed the check: the sample reads a
random subset, different on every execution, so a row count recorded under it cannot be reused - and the
storage aggregate is described as the scan it embeds, so it inherited the sample. A physical file scan
with a sample was likewise accepted, because the check also only looked at the logical file scan.

A table sample now invalidates the description for both scan classes, and a sampled `PhysicalFileScan` is
treated like a snapshot or scan params (unsupported). The reason is reported as
`tablesample of <table>`, so no struct info, fingerprint, pinned or learned entry is generated for such a
sub tree.

### Release note

An hbo statistics entry is no longer injected for a query which reads a table sample, whatever the scan
class or the operator which embeds it.

### Check List (For Author)

- Test: Unit Test
    - 203 cases of the stats package (including the new paired combinator case) plus the HBO command suite
      pass. A sampled physical scan case needs the olap scan fixture and is a follow-up.
- Behavior changed: Yes - a sampled physical scan (and a sampled physical file scan) is refused.
- Does this need documentation: No
… its columns

### What problem does this solve?

Problem Summary: the key of an outer join expansion appended the columns each side produces
(`sides:[t.k,t.v|u.k,u.w]`) to say which relation is on the preserved side. Column pruning pushes a
key-only project below the join and the project elimination passes keep it, so the same join on the same
equality read `sides:[t.k,t.v|u.k,u.w]` with `SELECT *` and `sides:[t.k|u.k]` for a key-only selection:
the equality population is identical, but a factor copied from one EXPLAIN missed the other.

An outer join now orders the operands of each equality by side - the operand of the left child is written
first - which names the preserved side without any output column, so column pruning cannot change the key
anymore. A pair which cannot name a side has no key: both operands from one child (the mirror plan applies
the condition inside the other child) and a pair whose operands are the same column (the two occurrences
are interchangeable in the key although only one of them is preserved) are refused, as the factor of such
a join would describe an orientation it cannot tell apart. The `sides` field, `sideOperands` and the output
column scan are gone.

### Release note

An hbo join expansion entry of an outer join is keyed by the join type and the side ordered equality
pairs: it no longer changes under column pruning, and an outer join whose equality pair reads one child
only has no key.

### Check List (For Author)

- Test: Unit Test
    - Paired tests pin the side ordered keys, the two refusal cases, and that a narrow (key-only) project
      below each child keeps the key; 205 cases of the stats package plus the HBO command suite pass.
- Behavior changed: Yes - the outer join expansion key loses its output columns and refuses a pair
  without a side.
- Does this need documentation: No
…ead lock

### What problem does this solve?

Problem Summary: the freshness comparison of a pinned entry called `table.getIndexIdList()` to know
whether the table has a rollup, without the table read lock. The list is a filtered copy of OlapTable's
plain index hash map, which a concurrent drop of a materialized view rewrites under the write lock, so an
HBO SHOW or DELETE STALE could throw ConcurrentModificationException and abort.

The index count is captured under the table read lock, which is released before the visible version rpc.

### Release note

HBO SHOW / HBO DELETE STALE no longer fail when a rollup or materialized view is dropped concurrently.

### Check List (For Author)

- Test: Unit Test
    - 205 cases of the stats package plus the HBO command suite pass; a coordinated reader/DDL case is a
      follow-up.
- Behavior changed: No
- Does this need documentation: No
… not loaded

### What problem does this solve?

Problem Summary: after a restart the pinned entries are loaded from the internal table. When that load
fails (the backend or the cloud compute group is unavailable), `ensurePinnedLoaded` returned normally,
so the admin caller presented the empty or partial cache as the complete set: HBO SHOW reported no stored
entries and HBO DELETE STALE acknowledged success without inspecting or deleting the persisted stale ones,
which the loader then installed later.

`ensurePinnedLoaded` now reports whether the required load completed, and `getAllPinnedPlanStatistics`
(the admin entry point) raises an error while it has not; the background loader keeps its retry loop.

### Release note

HBO SHOW and HBO DELETE STALE report an error instead of an empty result while the pinned statistics of
the FE could not be loaded.

### Check List (For Author)

- Test: Unit Test
    - 205 cases of the stats package plus the HBO command suite pass; a load failure case needs the
      internal table fixture and is a follow-up.
- Behavior changed: Yes - an incomplete initial load is an error for the admin statements.
- Does this need documentation: No
### What problem does this solve?

Problem Summary: the pinned entries are persisted with `StatisticsUtil.escapeSQL`, which doubles a
backslash assuming the standard sql mode, but the internal session of `execUpdate` inherited the global
default: with `NO_BACKSLASH_ESCAPES` a table name `t\q` was stored with two literal backslashes while the
in-memory entry kept one. After a restart the doubled name was registered as the relation key of that row,
and the identical live query was rejected before the fingerprint lookup.

The internal session now clears `NO_BACKSLASH_ESCAPES`, so both the write and every other internal
statement read the escaped values the way `escapeSQL` produces them.

### Release note

A pinned hbo entry whose structure contains a backslash (a quoted identifier, for example) keeps working
after an FE restart with NO_BACKSLASH_ESCAPES in the global sql_mode.

### Check List (For Author)

- Test: Unit Test
    - 205 cases of the stats package plus the HBO command suite pass; a restart case with a backslash
      identifier needs the persistence fixture and is a follow-up.
- Behavior changed: Yes - the internal session sql mode no longer inherits NO_BACKSLASH_ESCAPES.
- Does this need documentation: No
…ensitive

### What problem does this solve?

Problem Summary: the expression identity appends the session time zone when a function's value depends on
it, but the detection is a list plus a date-type check: a function whose argument and result are both not
date like slips through. `from_unixtime` is BIGINT to VARCHAR, so `Filter(Like(from_unixtime(t.ts),
'1970-01-01 00:%'))` over `ts = 0..3599` keeps 3600 rows in +00:00 and none in +08:00, yet the two plans
rendered one identity and one fingerprint: a pin copied in one session applied in the other, and the
unchanged scan baseline passed freshness. `current_time` returns TIME, which is not date like either,
although the session zone decides which wall clock it reads.

Both names are now in the session dependent list. The other epoch and time functions were audited:
`hour/minute/second/microsecond_from_unixtime` and `unix_timestamp` were already listed;
`now`/`current_timestamp`/`localtime` return DATETIME, and the `*_second_timestamp` family takes or returns
a date like type, so the type check covers them; `utc_time`/`utc_timestamp` read UTC and do not depend on
the session zone; `sec_to_time`, `time_to_sec`, `time_format`, `make_time`, `time_diff`, `add_time` and
`sub_time` operate on a TIME value and never read the zone.

### Release note

An hbo statistics entry of a plan whose predicate formats an epoch (`from_unixtime`) or reads the wall
clock (`current_time`) is no longer shared between sessions in different time zones.

### Check List (For Author)

- Test: Unit Test
    - A paired test pins that the same `from_unixtime` predicate has different fingerprints in +00:00 and
      +08:00; 206 cases of the stats package plus the HBO command suite pass. The `current_time` pair needs
      a TIME column fixture and is a follow-up.
- Behavior changed: Yes - two more functions carry the session time zone in their identity.
- Does this need documentation: No
… expression

### What problem does this solve?

Problem Summary: the session locale decides the day and month names a function emits, but the expression
identity only carried the session time zone. `Filter(dayname(t.d) = 'Monday')` over 1000 rows of
2024-01-01 and 1000 rows of 2024-01-02 keeps 1000 rows with lc_time_names='en_US' and none with 'de_DE',
yet both sessions rendered one identity and one fingerprint: the unchanged scan baseline passed freshness
and a pin copied in the English session overrode the German estimate.

`dayname` and `monthname` are now locale sensitive by name, and a format based conversion (`date_format`,
`from_unixtime`, `str_to_date`, `time_format`) is locale sensitive when its format argument names a month
or a weekday (`%M`, `%W`, `%a`, `%b`) or cannot be inspected because it is computed at run time; a call
without a format argument keeps its key. The session locale is appended to the identity next to the time
zone (`DayName<de_DE>`, `FromUnixtime<+08:00|en_US>`), read from the session which plans the query.

### Release note

An hbo statistics entry of a plan whose expression prints a day or month name is no longer shared between
sessions with different lc_time_names.

### Check List (For Author)

- Test: Unit Test
    - A paired test pins that `dayname(d) = 'Monday'` has different fingerprints with en_US and de_DE;
      207 cases of the stats package plus the HBO command suite pass. The pin reuse pair and the format
      argument cases need their own fixtures and are follow-ups.
- Behavior changed: Yes - a locale sensitive function carries the session locale in its identity.
- Does this need documentation: No
@englefly
englefly dismissed stale reviews from morningman and 924060929 via cda45a6 November 12, 2026 09:38
@englefly
englefly force-pushed the hbo-structinfo-squashed branch from 0f40234 to cda45a6 Compare November 12, 2026 09:38
@englefly

Copy link
Copy Markdown
Contributor Author

run buildall

@englefly

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of head cda45a6c16479290eeb6af40ef6d44afff9a2dca: two new P2 findings are included inline. They concern VARIANT field identity in join expansion and quoted-alias corruption of scan baselines. Existing discussions and replies were checked against all 95 live inline comments; known concerns were suppressed, including author-declined ones. No existing P0/P1 finding was present to carry forward.

The two complete reviewers and separate risk reviewer covered the authoritative changed-file list, related callers, tests and special paths. Each returned NO_NEW_VALUABLE_FINDINGS in the second round after the two accepted findings were independently verified. Every candidate and risk focus has an accepted, duplicate or evidence-based dismissal disposition; no separate question or assigned coverage gap remains. Convergence describes completion of the review process; the reported defects still need attention.

Critical checkpoint conclusions:

  • Goal and proof: structural HBO keys, pinned/learned lookup, equality expansion, persistence and EXPLAIN/SHOW are wired end to end. The two new defects and previously reported limitations prevent treating the feature as fully correct. Test sources support nominal behavior; they were not executed here.
  • Scope and clarity: the broad FE change is focused on HBO and preserving its identity through physical-plan rebuilds. All 117 diff sections, including eight unchanged legacy test moves, were covered.
  • Concurrency: planner-local memo state, daemon loading, connection-thread administration, profile publication, config callbacks and cross-FE SQL writes were traced. The local load/write monitor does not block ordinary cache-only planning. Existing metadata-snapshot and cross-FE replacement concerns remain known; no additional lock cycle was established.
  • Lifecycle: the loader retries initial failures, responds to persistence enablement, and exits after completion or disabled internal schema. Descriptors live with query memo groups; copied mutable-state holders are independent. Known relation-key retention remains covered by its existing thread.
  • Configuration: row-drift and FILTER_SMALL thresholds are read at lookup; persistence is hot mutable with documented one-time loading; pinned cache size is startup-only. The existing relation-table-cache callback omission was suppressed.
  • Compatibility: the structural key replaces the prior HBO identity. The internal SQL table validates required columns and refuses incompatible old schemas. No additional mixed-version protocol defect was established statically.
  • Thrift/protobuf IDs: no gensrc/thrift or gensrc/proto definitions change, so the cross-branch field-ID checkpoint is not applicable.
  • Parallel paths: logical/physical OLAP scans, file scans, storage wrappers, partial aggregation, exact/agnostic modes, pinned/learned precedence and the CostModel consumer were traced. Previously raised physical-tablet and learned-skew freshness issues were not reposted.
  • Conditions and errors: unsupported shapes/selectors, invalid parameters, numeric exactness, missing structures, persistence failures and initial-load failures were checked. Existing stale-cleanup error acknowledgment and expansion early-return/selectivity concerns remain known.
  • Test coverage: added unit, golden and regression sources cover nominal identity, modes, guards, drift, commands and persistence. The new findings lack the relevant field-pair and quoted-alias cases. Restart/failover and coordinated races were inspected through control flow only.
  • Expected results: committed golden/captured data and the persistence .out were inspected. The known equality-only expansion assertion mismatch and row-report timing issue remain in existing threads. No expected output was generated or certified by execution.
  • Observability: EXPLAIN applied/skipped markers, SHOW baseline/detail and persistence/loader logs expose the intended decisions. The alias finding makes SHOW disagree with query-side freshness; existing cleanup failure visibility remains known.
  • Transactions, persistence and failover: ordinary internal-table SQL supplies storage without a new EditLog operation. Local SQL-before-cache ordering and putIfAbsent load precedence were checked. Existing cross-FE generation loss and DELETE visibility limitations remain covered by prior comments.
  • Data writes and atomicity: writes concern HBO caches and its internal statistics table; user transaction commit/publish and delete-bitmap code are unchanged. No additional crash-induced leak, deadlock or user-data loss was established in the changed paths.
  • FE/BE variables: runtime feedback uses existing fields/counters; hbo_max_scans_per_group is forwarded and fingerprint display stays in FE EXPLAIN. No new transmitted field or missing BE consumer was found.
  • Performance and memory: scan-count/empty-cache/relation gates and memoized descriptors/renderings reduce key work; planning avoids internal SQL. Existing relation-index growth and cache-reconfiguration concerns were suppressed. No benchmark or runtime memory validation was performed.
  • Other concerns and user focus: producer binding, volatile sharing, semantic expression fields, operand encoding and learned input alignment were rechecked against prior discussions. The tentative nested-namespace/dotted-column case was retained as evidence under the existing operand-encoding concern, not posted again. No additional user focus was supplied.

Validation was static only: no builds, engine/JVM tests, regression execution, benchmarks, source edits or submodule initialization. All 18,028 right-side lines in the authoritative hunks match the current checkout. Live PR base/head and the unchanged working tree were rechecked before delivery.

Review converged after 2 round(s).

}
return Optional.of(escapeOperandComponent(
HboScanDescriptor.qualifiedNameOf(slot.getOriginalTable().get()))
+ "." + escapeOperandComponent(slot.getOriginalColumn().get().getName()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve VARIANT subpaths in expansion operands

The default subpath rewrite turns these supported reduced plans into direct slot equalities:

INNER(l.v['a'] = r.v['a'], Scan(l), Scan(r))
INNER(l.v['b'] = r.v['b'], Scan(l), Scan(r))

The extracted slots retain original column v, so this return gives both joins the same JE fingerprint. Give each table {a:1,b:1},{a:1,b:2} and use supported ALTER TABLE ... MODIFY COLUMN v SET STATS ('row_count'='2','ndv'='2','num_nulls'='0'). StatsCalculator fetches those known statistics by base name v, so both equality estimates are two. A factor of two correctly gives the a join four rows, but applyPinnedJoinExpansion also applies it to the b join, which returns two; the Cartesian cap permits four. Include the counted subpath in the operand identity, or decline such operands. This is separate from the earlier GroupStructInfo subpath fix: this auxiliary expansion lookup still drops the entire path.

// '#' introduces the partition / tablet / index selector markers of a scan token, so a table name
// which carries one would be truncated to the part before it while the planner summary keeps the
// whole name - the relation key of the entry would then never match the query
return name.replace("%", "%25").replace(".", "%2E").replace("#", "%23")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Escape commas in aliases before rendering scan metadata

For the full-table plan Filter(predicate) -> Scan(t AS alias) with quoted alias x,p1/2, the alias reaches this codec unchanged and produces a token such as S{internal.db.t#ti:ID#ra:x,p1/2,v2,r1000}. parse() splits on commas, truncates the alias to x, and invents a partial-partition baseline (selected=1,total=2); a full scan writes no real p field to replace it. Pin the advertised key/struct, then insert 10 rows and obtain comparable reported rows at v3/r1010. Query-side between/stateOf still applies the pin as DRIFTED, but catalog-side of() treats the false partial selection as UNKNOWN. Once the entry has positive age, HBO DELETE STALE STATISTICS OLDER_THAN 0 therefore removes this usable pin; an ordinary full-table alias remains DRIFTED and is retained. Escape the scan-token comma separator in alias components and cover the descriptor round trip and cleanup behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants