Skip to content

[fix](be) Fix TIMESTAMPTZ nullable type equality - #66722

Merged
HappenLee merged 1 commit into
apache:masterfrom
Mryange:fix-timestamptz-type-equals
Aug 19, 2026
Merged

[fix](be) Fix TIMESTAMPTZ nullable type equality#66722
HappenLee merged 1 commit into
apache:masterfrom
Mryange:fix-timestamptz-type-equals

Conversation

@Mryange

@MryangeMryange commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem Summary:
A TIMESTAMPTZ data type was considered equal to its nullable variant because the equality check only compared the primitive type and scale. During constant folding of a STRUCT expression, this caused a type mismatch when a STRUCT<TIMESTAMPTZ> constant was assigned to a STRUCT<Nullable(TIMESTAMPTZ)> expression. The equality check now also requires the concrete data type to match, preventing the invalid equivalence while preserving scale-sensitive comparisons. Regression coverage verifies inserting a constant TIMESTAMPTZ value into a STRUCT column and reading the normalized UTC value.

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

@hello-stephen

hello-stephen commented Aug 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?

@Mryange

Mryange commented Aug 13, 2026

Copy link
Copy Markdown
ContributorAuthor

/review

github-actions[bot]
github-actionsBot reviewed Aug 13, 2026
View reviewed changes

@github-actionsgithub-actionsBot 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.

Automated review opinion: no blocking or material correctness issue was found. The change is suitable to merge once the required CI checks complete.

Critical checkpoint conclusions:

  • Goal and proof: the old DataTypeTimeStampTz::equals treated TIMESTAMPTZ(6) as equal to Nullable(TIMESTAMPTZ(6)), making recursive Struct equality asymmetric and allowing a cast identity path to retain the wrong physical child-column shape. Exact dynamic type plus scale restores the intended equality contract. The unit test proves scale sensitivity, both nullable operand orders, and recursive Struct behavior; the regression proves the insert/read result.
  • Scope and clarity: the production change is a focused equality-predicate correction and mirrors the established TIMEV2/DATETIMEV2 implementation. No additional production site needs modification.
  • Concurrency: datatype metadata is immutable and this comparison adds no shared mutation, thread interaction, lock, or atomic state.
  • Lifecycle and ownership: no lifecycle or static-initialization behavior changes. The corrected comparison prevents an invalid identity cast from sharing a non-nullable child under a nullable target; the existing Struct cast now constructs the required nullable child.
  • Configuration: no configuration item or dynamic-update behavior is added or changed.
  • Compatibility: there is no symbol, wire protocol, storage layout, serialized block, or persisted metadata change, so no rolling-upgrade compatibility branch is needed.
  • Parallel and special-case paths: scalar, NULL, multi-field, nested Array/Map/Struct, scale-changing, non-folded, cloud, and shared-nothing paths were traced. Recursive equality/cast wrappers propagate the leaf fix, and precision-ignoring callers explicitly remove nullable wrappers or execute after nullable unnesting.
  • Test coverage: the BE unit test directly covers the faulty equality boundary. The regression reaches ordinary BE constant materialization through VExpr::open/get_const_col and the sink Struct cast even without enabling the optional FE-to-BE fold RPC. Nearby TIMESTAMPTZ suites cover additional complex-type, NULL, scale, and timezone behavior.
  • Test results: the generated expected value is correct and deterministic: 2024-03-30 20:45:00 -03:30 normalizes to 2024-03-31 00:15:00.000000+00:00. The test follows the table-cleanup, hardcoded-name, ordered-query, and generated-output conventions. Per the review-runner instruction, I did not run builds or tests; formatter/checkstyle/license and other lightweight PR checks were passing at review time, while BE UT was still pending.
  • Error handling and observability: no Status or exception path is weakened. Existing cast/type checks provide failure context, and this local metadata correction does not require new logs, metrics, or user-facing diagnostics.
  • Transactions, persistence, and writes: the regression exercises a real insert/read, but transaction atomicity, visible versions, EditLog, rowsets, delete bitmaps, and crash recovery are unchanged.
  • FE-BE state: no new variable or metadata field is transmitted; existing Struct nullability and TIMESTAMPTZ scale metadata are sufficient.
  • Performance and memory: the comparison replaces primitive/scale virtual calls with one RTTI comparison and a scale load. The nullable child allocation selected for the corrected cast is required output materialization; no redundant hot-path work or untracked ownership was introduced.
  • Other issues and user focus: no additional review focus was provided, and the full changed-file sweep found no other substantiated issue.

Review completion: the review converged after round 1. Two full-review subagents and one separate risk-focused subagent all returned NO_NEW_VALUABLE_FINDINGS; every initial risk was independently resolved, there were no existing review threads to duplicate, and no inline comment is warranted.

@Mryange

Mryange commented Aug 13, 2026

Copy link
Copy Markdown
ContributorAuthor

run buildall

@hello-stephen

hello-stephen commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (2/2) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage76.14% (32587/42798)
Line Coverage61.07% (366189/599640)
Region Coverage57.75% (308269/533756)
Branch Coverage59.13% (139926/236650)

@Mryange

Mryange commented Aug 13, 2026

Copy link
Copy Markdown
ContributorAuthor

run performance

1 similar comment
@Mryange

Mryange commented Aug 17, 2026

Copy link
Copy Markdown
ContributorAuthor

run performance

@Mryange

Mryange commented Aug 17, 2026

Copy link
Copy Markdown
ContributorAuthor

run buildall

HappenLee
HappenLee approved these changes Aug 18, 2026
View reviewed changes

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

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Aug 18, 2026
@github-actions

github-actionsBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actionsBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@Mryange

Mryange commented Aug 18, 2026

Copy link
Copy Markdown
ContributorAuthor

run buildall

@hello-stephen

hello-stephen commented Aug 18, 2026

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 17047 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit ef57180017b2922d930c0b9f1f3d303f30dcfb3d, data reload: false
------ Round 1 ----------------------------------
orders	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL ============================================
q1	17631	3074	3041	3041
q2	q3	10862	863	499	499
q4	4681	251	202	202
q5	7675	565	418	418
q6	140	118	91	91
q7	523	510	392	392
q8	9257	947	892	892
q9	3509	2404	2387	2387
q10	6570	894	727	727
q11	461	262	248	248
q12	691	401	329	329
q13	17869	1552	1166	1166
q14	168	151	137	137
q15	q16	488	388	366	366
q17	807	800	736	736
q18	3126	2320	2235	2235
q19	1143	869	688	688
q20	729	538	430	430
q21	5341	1833	1947	1833
q22	338	266	230	230
Total cold run time: 92009 ms
Total hot run time: 17047 ms
----- Round 2, with runtime_filter_mode=off -----
orders	Doris	NULL	NULL	150000000	42	6422171781	NULL	22778155	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL ============================================
q1	3406	3342	3327	3327
q2	q3	2208	2346	2149	2149
q4	1201	1176	894	894
q5	2192	2108	2094	2094
q6	172	120	90	90
q7	1029	918	863	863
q8	1606	1407	1434	1407
q9	3142	3112	3105	3105
q10	1836	1777	1625	1625
q11	372	280	262	262
q12	460	423	345	345
q13	1533	1539	1170	1170
q14	168	181	158	158
q15	q16	400	387	366	366
q17	1049	1040	1034	1034
q18	4991	4401	4797	4401
q19	840	844	858	844
q20	976	921	793	793
q21	3501	3275	3252	3252
q22	404	347	315	315
Total cold run time: 31486 ms
Total hot run time: 28494 ms

@hello-stephen

hello-stephen commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (2/2) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage75.73% (34078/44997)
Line Coverage61.21% (386850/631986)
Region Coverage57.76% (326744/565670)
Branch Coverage58.75% (149416/254317)

@hello-stephen

hello-stephen commented Aug 18, 2026

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 81899 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 ef57180017b2922d930c0b9f1f3d303f30dcfb3d, data reload: false
query5	4270	433	351	351
query6	417	167	157	157
query7	4840	458	262	262
query8	309	123	119	119
query9	8681	2929	2931	2929
query10	397	258	226	226
query11	5367	1026	939	939
query12	117	72	70	70
query13	1187	454	329	329
query14	6128	2238	2127	2127
query14_1	2012	2023	2029	2023
query15	186	118	116	116
query16	956	383	364	364
query17	854	441	349	349
query18	2335	333	232	232
query19	176	137	110	110
query20	68	67	68	67
query21	214	116	102	102
query22	5455	5275	5280	5275
query23	6802	6120	5875	5875
query23_1	6022	6196	5938	5938
query24	7366	1115	774	774
query24_1	777	762	775	762
query25	414	290	243	243
query26	1252	263	155	155
query27	2739	438	272	272
query28	4654	1515	1494	1494
query29	937	426	345	345
query30	273	178	152	152
query31	854	432	369	369
query32	106	53	52	52
query33	482	229	184	184
query34	1045	832	478	478
query35	414	404	358	358
query36	588	559	546	546
query37	130	85	73	73
query38	1102	847	813	813
query39	530	501	494	494
query39_1	461	471	464	464
query40	226	128	118	118
query41	58	56	59	56
query42	81	81	82	81
query43	246	247	217	217
query44 query45	117	110	98	98
query46	789	877	534	534
query47	786	792	736	736
query48	311	301	235	235
query49	541	225	190	190
query50	806	330	266	266
query51	8355	8216	8764	8216
query52	74	77	68	68
query53	210	235	203	203
query54	250	179	168	168
query55	72	59	56	56
query56	221	218	223	218
query57	698	634	625	625
query58	245	183	182	182
query59	1224	1245	1123	1123
query60	299	218	203	203
query61	148	142	133	133
query62	373	230	176	176
query63	197	161	162	161
query64	2884	786	702	702
query65 query66	2042	310	263	263
query67	10192	10250	10279	10250
query68 query69	361	243	213	213
query70	668	632	628	628
query71	307	254	271	254
query72	2401	1852	1673	1673
query73	692	592	340	340
query74	1574	1272	1186	1186
query75	1312	1197	1104	1104
query76	2427	746	558	558
query77	295	268	218	218
query78	4186	3726	3379	3379
query79	3815	769	582	582
query80	1778	435	384	384
query81	529	202	176	176
query82	765	142	113	113
query83	336	259	239	239
query84 query85	896	447	380	380
query86	518	175	162	162
query87	1049	1035	935	935
query88	4413	2165	2146	2146
query89	302	230	209	209
query90	2090	150	145	145
query91	155	147	125	125
query92	67	47	43	43
query93	2929	1208	821	821
query94	680	276	243	243
query95	629	373	355	355
query96	789	577	275	275
query97	1129	1154	1066	1066
query98	187	135	141	135
query99	475	355	317	317
Total cold run time: 179537 ms
Total hot run time: 81899 ms

@hello-stephen

hello-stephen commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (2/2) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage75.73% (34078/44997)
Line Coverage61.21% (386850/631986)
Region Coverage57.76% (326744/565670)
Branch Coverage58.75% (149416/254317)

@hello-stephen

hello-stephen commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (2/2) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage75.98% (34205/45021)
Line Coverage61.35% (388265/632901)
Region Coverage57.89% (328159/566885)
Branch Coverage58.87% (149945/254704)

@hello-stephen

hello-stephen commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (2/2) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage74.92% (33730/45021)
Line Coverage59.91% (379169/632901)
Region Coverage56.06% (317787/566885)
Branch Coverage56.90% (144936/254704)

@hello-stephen

hello-stephen commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (2/2) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage74.97% (33753/45021)
Line Coverage59.94% (379363/632901)
Region Coverage56.06% (317769/566885)
Branch Coverage56.94% (145019/254704)

@HappenLee
HappenLee merged commit a21b5f2 into apache:masterAug 19, 2026
33 checks passed
github-actionsBot pushed a commit that referenced this pull request Aug 19, 2026
@MryangeYour Name
[fix](be) Fix TIMESTAMPTZ nullable type equality (#66722)
686021f
A TIMESTAMPTZ data type was considered equal to its nullable variant
because the equality check only compared the primitive type and scale.
During constant folding of a STRUCT expression, this caused a type
mismatch when a `STRUCT<TIMESTAMPTZ>` constant was assigned to a
`STRUCT<Nullable(TIMESTAMPTZ)>` expression. The equality check now also
requires the concrete data type to match, preventing the invalid
equivalence while preserving scale-sensitive comparisons. Regression
coverage verifies inserting a constant TIMESTAMPTZ value into a STRUCT
column and reading the normalized UTC value.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
@github-actionsgithub-actionsBot mentioned this pull request Aug 19, 2026
Merged
yiguolei pushed a commit that referenced this pull request Aug 20, 2026
@github-actions@Mryange
branch-4.1: [fix](be) Fix TIMESTAMPTZ nullable type equality#66722(#…
6bf7b36
…66945)
Cherry-picked from #66722
Co-authored-by: Mryange <yanxuecheng@selectdb.com>
@yiguoleiyiguolei mentioned this pull request Sep 1, 2026
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actionsgithub-actions[bot]github-actions[bot] left review comments
@HappenLeeHappenLeeHappenLee approved these changes

Assignees

No one assigned

Labels

approvedIndicates a PR has been approved by one committer.dev/4.1.4-mergedreviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Mryange@hello-stephen@HappenLee@yiguolei