Skip to content

[fix](point query) Fix BE crash when a point query reads a column added by light schema change - #68997

Merged
yiguolei merged 1 commit into
apache:masterfrom
csun5285:fix/point-query-slot-column
Oct 14, 2026
Merged

[fix](point query) Fix BE crash when a point query reads a column added by light schema change#68997
yiguolei merged 1 commit into
apache:masterfrom
csun5285:fix/point-query-slot-column

Conversation

@csun5285

@csun5285 csun5285 commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Since #68198, a short-circuit point query that reads a MAP or STRUCT column added by light schema change, before any new load, aborts the BE. tablet_fetch_data now also catches exceptions, so a point query that throws returns an error instead.

CREATE TABLE t (k INT NOT NULL, stored INT NOT NULL, wide INT NOT NULL)
UNIQUE KEY(k) DISTRIBUTED BY HASH(k) BUCKETS 1
PROPERTIES ("replication_num" = "1", "enable_unique_key_merge_on_write" = "true",
            "light_schema_change" = "true", "store_row_column" = "true", "row_store_columns" = "k,stored");
INSERT INTO t VALUES (1, 10, 100);
ALTER TABLE t ADD COLUMN obj MAP<STRING, ARRAY<INT>> NULL;
SET enable_short_circuit_query = true;
SET enable_short_circuit_query_access_column_store = true;
SELECT obj, stored, wide FROM t WHERE k = 1;
-- before: BE aborts, client gets RpcException ... UNAVAILABLE: io exception
-- after:  NULL 10 100

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@csun5285 @claude
[fix](point query) Fix BE crash when a point query reads a MAP or STR…
6dca50d 
…UCT column added by light schema change

### What problem does this PR solve?

Issue Number: close #xxx

Related PR: apache#68198

Problem Summary:

A light schema change adds a column without writing any data, so until the next load the BE's
tablet schema does not have it. A short-circuit point query that reads such a column from the
column store (the column is outside `row_store_columns`) builds the column from the query slot.
apache#68198 built it with `variant_util::get_column_by_type`, which is meant for variant subcolumns
and has no branch for MAP or STRUCT. It throws, and `tablet_fetch_data` does not catch
exceptions the way the other RPCs do, so the BE aborts:

    terminate called after throwing an instance of 'doris::Exception'
      what():  [E6] unexcepted data column type: Map(Nullable(String), Nullable(Array(Nullable(INT)))), column name is: obj

Reproduce on a merge-on-write table with "store_row_column" = "true" and
"row_store_columns" = "k,stored":

    INSERT INTO t VALUES (1, 10, 100);
    ALTER TABLE t ADD COLUMN obj MAP<STRING, ARRAY<INT>> NULL;  -- wait until FINISHED
    SET enable_short_circuit_query = true;
    SET enable_short_circuit_query_access_column_store = true;
    SELECT obj, stored, wide FROM t WHERE k = 1;

Before: the BE aborts and the client gets `RpcException ... UNAVAILABLE: io exception`.
After: `NULL 10 100`, the same as a normal scan.

The fix:

1. Add `ReadSchema::tablet_column_from_slot`, which builds the TabletColumn a slot reads from the
   slot's name, unique id, type and default value. It covers every type the slot can carry;
   ARRAY, MAP and STRUCT get their sub columns, named the way FE names them
   (item / key / value / field name). The point query uses it instead of
   `variant_util::get_column_by_type`.
2. `_tablet_fetch_data` turns an exception into an error status, so a point query that throws
   fails instead of taking the BE down.

### Release note

Fix BE crash when a short-circuit point query reads a MAP or STRUCT column that a light schema
change added before any new data was loaded.

### Check List (For Author)

- Test: Regression test / Unit Test / Manual test
    - Regression test: point_query_p0/test_point_query_light_schema_change now also adds MAP,
      STRUCT and ARRAY columns. It passes with the fix; without it the BE aborts at the new
      `added_complex_columns` query. All 17 point_query_p0 suites pass with the fix.
    - Unit Test: 3 new ReadSchemaTest cases for tablet_column_from_slot.
    - Manual test: the SQL above on an ASAN build, before and after the fix. For 2, a build that
      still uses the old column builder returns the error to the client and the BE keeps running.
- Behavior changed: No
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@hello-stephen

hello-stephen commented Oct 13, 2026

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?

@csun5285

csun5285 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

/review

github-actions[bot]
github-actions Bot reviewed Oct 13, 2026
View reviewed changes

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

Completed static review of all seven changed files at commit 6dca50d. No new actionable findings and no existing P0/P1 inline findings to carry forward. The change reconstructs the declared type of a column added by light schema change so a point lookup before the next load can use the segment's default iterator, including MAP/STRUCT/ARRAY, while converting Doris exceptions to an RPC error status.

Critical checkpoint conclusions:

  • Goal and proof: FE slot -> recursive TabletColumn reconstruction -> missing segment UID -> default iterator implements the intended pre-first-load behavior by inspection. Added unit and regression tests exercise the conversion and complex-column reads; execution was not performed.
  • Scope and reuse: the seven-file change is focused; one caller uses a shared ReadSchema helper and existing type/field-length utilities.
  • Concurrency and locks: independent RPC callbacks build request-local metadata. Tablet schema snapshots retain shared ownership under the existing metadata lock, and the reusable block pool retains its mutex. No new shared mutation, lock order or heavy locked operation was introduced.
  • Lifecycle and static initialization: children have owned storage without ownership cycles. Failed initialization does not publish a partial Reusable; failed lookup destroys its partial block rather than pooling it. Scoped COW restoration and rowset/cache RAII preserve cleanup. No new production global initializer exists.
  • Configuration: no new setting; the existing column-store access flag keeps its gate.
  • Compatibility and rolling upgrades: existing FE slot metadata is consumed without wire or persisted-format changes. No new runtime library, linker or glibc API dependency was introduced; binary compatibility was not tested.
  • Thrift/protobuf IDs: no gensrc definitions changed, so release-branch field-ID comparisons are inapplicable.
  • Parallel paths: inspected schema-present/absent columns, full/partial row-store, cloud/local segment reads and the separate VARIANT access path. Existing paths retain their behavior; current FE short-circuit eligibility excludes VARIANT tables.
  • Conditions and error handling: the absent-column condition has the concrete pre-load schema-gap cause documented at the caller. Non-OK Status remains propagated; both catch boundaries convert Doris exceptions. FE checks response status before accepting any batch. Nullability and typed defaults follow the existing iterator.
  • Test coverage: three helper tests cover recursive children, scalar/complex round trips, string lengths, DECIMALV2 declaration details and VARIANT parameters. Regression additions cover MAP/STRUCT/ARRAY before load, old rows after load and populated new rows. Dedicated exception-injection, concurrency, prepared-cache and cloud tests are absent; no concrete new defect was substantiated from those gaps.
  • Changed results: every added golden value agrees with the SQL by inspection. Each new query selects one full unique key, so row order is deterministic. Output generation provenance was not independently verified and outputs were not regenerated.
  • Observability: existing column-identifying errors and point-query timing/profile remain sufficient for this local conversion; no new distributed operation or background state needs instrumentation.
  • Transactions and persistence: no production metadata writes, EditLog, publish or failover path changed.
  • Writes and atomicity: production write behavior is unchanged; a failed lookup returns an error and FE does not consume a partial batch.
  • FE/BE variables: no new transmitted field; name, unique ID, full nested type/nullability and default use the existing descriptor path. Prepared query context invalidation on schema version change remains in place.
  • Performance: reconstruction is linear in the type tree and limited to columns absent from the tablet schema. Missing-UID default reads add no data-column page IO. No new scan, large buffer or lock contention was identified.
  • Other invariants and BE hygiene: rowset/version selection, MoW delete bitmap and physical segment IDs are unchanged. The text-only build-hygiene command passed header layering/closure/reach/PCH, extern-template pairing and unity-skip coverage. No builds, unit/regression tests, formatting mutation or source changes were performed.

No additional user focus was supplied. Initial risks covering nested type fidelity, default metadata and exception cleanup were independently investigated and dismissed with code evidence. Two normal reviewers and a separate risk reviewer all returned NO_NEW_VALUABLE_FINDINGS in round 1; the main reconciliation and final changed-file/unresolved-point sweep found no unresolved candidate. Review complete.

@csun5285

csun5285 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

hello-stephen commented Oct 13, 2026

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 29066 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 6dca50d8f2406f55a9885de1006606afaf206391, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17611	5623	5571	5571
q2	2080	328	308	308
q3	10186	1408	849	849
q4	4676	472	346	346
q5	7452	794	550	550
q6	186	188	156	156
q7	773	807	624	624
q8	9244	1380	1071	1071
q9	5795	4534	4534	4534
q10	6825	1312	1041	1041
q11	439	259	239	239
q12	635	406	289	289
q13	18073	3117	2375	2375
q14	272	276	253	253
q15	q16	745	741	676	676
q17	1200	748	570	570
q18	7177	6261	6172	6172
q19	1116	953	614	614
q20	395	340	230	230
q21	5160	2328	2468	2328
q22	369	316	270	270
Total cold run time: 100409 ms
Total hot run time: 29066 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	6232	6254	6173	6173
q2	695	591	540	540
q3	5223	5402	4917	4917
q4	2061	2139	1557	1557
q5	5263	5438	5923	5438
q6	260	200	150	150
q7	2299	2019	1888	1888
q8	3221	2831	2779	2779
q9	8251	8302	8387	8302
q10	3759	3750	3379	3379
q11	669	464	462	462
q12	713	738	592	592
q13	2905	3228	2487	2487
q14	319	317	319	317
q15	q16	690	794	675	675
q17	8512	7559	7540	7540
q18	13198	12272	13131	12272
q19	960	815	819	815
q20	2200	2179	1966	1966
q21	6028	4989	5019	4989
q22	518	448	399	399
Total cold run time: 73976 ms
Total hot run time: 67637 ms

@hello-stephen

hello-stephen commented Oct 13, 2026

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 152764 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 6dca50d8f2406f55a9885de1006606afaf206391, data reload: false

query5	4332	585	444	444
query6	426	203	182	182
query7	4807	446	234	234
query8	322	182	166	166
query9	8735	3967	3992	3967
query10	473	325	286	286
query11	5917	3415	3221	3221
query12	141	88	82	82
query13	1244	433	334	334
query14	6534	4849	4506	4506
query14_1	4286	4312	4259	4259
query15	209	202	189	189
query16	929	432	437	432
query17	848	660	548	548
query18	2417	428	315	315
query19	193	172	152	152
query20	86	80	83	80
query21	216	146	113	113
query22	13009	13078	12843	12843
query23	13263	12687	12311	12311
query23_1	12386	12316	12194	12194
query24	7099	863	466	466
query24_1	481	481	479	479
query25	540	422	362	362
query26	1247	261	143	143
query27	2763	445	272	272
query28	4608	1957	1988	1957
query29	1553	597	472	472
query30	296	223	181	181
query31	892	756	654	654
query32	147	97	98	97
query33	506	307	266	266
query34	910	864	508	508
query35	759	763	641	641
query36	810	829	732	732
query37	137	107	93	93
query38	1798	1788	1745	1745
query39	699	705	665	665
query39_1	649	646	671	646
query40	223	123	98	98
query41	73	70	71	70
query42	87	94	87	87
query43	369	390	343	343
query44	1379	711	715	711
query45	184	179	161	161
query46	856	977	577	577
query47	3010	3055	2838	2838
query48	309	308	222	222
query49	560	392	289	289
query50	689	292	210	210
query51	10614	10545	10567	10545
query52	82	83	69	69
query53	190	219	157	157
query54	232	201	172	172
query55	71	72	62	62
query56	223	218	204	204
query57	1621	1580	1558	1558
query58	273	262	249	249
query59	2352	2379	2129	2129
query60	296	237	225	225
query61	152	147	147	147
query62	386	343	286	286
query63	205	170	172	170
query64	2735	956	777	777
query65	3479	3425	3392	3392
query66	1788	418	301	301
query67	20115	20084	20139	20084
query68	3042	1027	620	620
query69	379	296	253	253
query70	919	847	838	838
query71	304	241	211	211
query72	2790	2231	2301	2231
query73	543	540	294	294
query74	4604	4472	4305	4305
query75	2354	2302	1968	1968
query76	2308	1013	635	635
query77	370	392	305	305
query78	9089	9028	8493	8493
query79	980	895	546	546
query80	525	414	337	337
query81	538	323	270	270
query82	576	133	106	106
query83	307	202	197	197
query84	310	119	124	119
query85	807	425	372	372
query86	314	251	236	236
query87	2034	1991	1836	1836
query88	3656	2731	2710	2710
query89	343	305	266	266
query90	1897	186	185	185
query91	160	151	123	123
query92	101	89	86	86
query93	1008	994	566	566
query94	444	292	268	268
query95	585	418	318	318
query96	679	527	241	241
query97	2449	2429	2312	2312
query98	157	153	145	145
query99	778	778	653	653
Total cold run time: 234188 ms
Total hot run time: 152764 ms

@hello-stephen

hello-stephen commented Oct 13, 2026

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.64 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 6dca50d8f2406f55a9885de1006606afaf206391, data reload: false

query1	0.00	0.00	0.01
query2	0.10	0.05	0.04
query3	0.27	0.13	0.12
query4	1.62	0.12	0.12
query5	0.27	0.24	0.24
query6	1.17	0.69	0.68
query7	0.03	0.01	0.00
query8	0.05	0.04	0.04
query9	0.43	0.35	0.36
query10	0.58	0.56	0.60
query11	0.21	0.14	0.14
query12	0.19	0.15	0.14
query13	0.50	0.49	0.50
query14	0.97	0.97	0.97
query15	0.63	0.61	0.62
query16	0.31	0.35	0.34
query17	1.12	1.18	1.14
query18	0.23	0.21	0.21
query19	2.17	2.02	2.04
query20	0.02	0.01	0.02
query21	15.48	0.29	0.14
query22	4.51	0.06	0.05
query23	16.24	0.32	0.12
query24	2.95	0.87	0.43
query25	0.09	0.06	0.05
query26	1.81	0.23	0.15
query27	0.05	0.04	0.03
query28	2.01	0.57	0.41
query29	12.51	4.19	3.30
query30	0.29	0.15	0.16
query31	2.77	0.58	0.34
query32	3.22	0.61	0.51
query33	3.24	3.23	3.20
query34	15.34	3.95	3.39
query35	3.37	3.35	3.38
query36	0.55	0.45	0.49
query37	0.10	0.07	0.06
query38	0.05	0.04	0.04
query39	0.03	0.03	0.03
query40	0.16	0.14	0.14
query41	0.08	0.03	0.02
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 95.8 s
Total hot run time: 24.64 s

@yiguolei
yiguolei merged commit 310a884 into apache:master Oct 14, 2026
44 checks passed
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

1 similar comment
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

1 similar comment
@github-actions

github-actions Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@wenzhenghu wenzhenghu mentioned this pull request Nov 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments
@yiguolei yiguolei yiguolei approved these changes
@gavinchou gavinchou Awaiting requested review from gavinchou gavinchou is a code owner

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@csun5285 @hello-stephen @yiguolei