Skip to content

[Fix](nereids) Preserve negative zero sign in SIGNBIT constant folding#63954

Merged
zclllyybb merged 1 commit into
apache:masterfrom
linrrzqqq:fix-signbit-fold-constant
Jun 2, 2026
Merged

[Fix](nereids) Preserve negative zero sign in SIGNBIT constant folding#63954
zclllyybb merged 1 commit into
apache:masterfrom
linrrzqqq:fix-signbit-fold-constant

Conversation

@linrrzqqq
Copy link
Copy Markdown
Collaborator

Problem Summary:

signbit in Nereids FE constant folding used value < 0 to determine the sign, which treats -0.0 as non-negative and folds it to false. This is inconsistent with:

  • the runtime BE implementation, which uses std::signbit
  • the documented signbit behavior, which distinguishes +0.0 and -0.0

+0.0 and -0.0 compare equal numerically, so value < 0 cannot distinguish them. Their difference is only recorded in the floating-point sign bit. Using raw bits makes FE constant folding consistent with BE runtime semantics.

before:

Doris> set debug_skip_fold_constant=true;
Query OK, 0 rows affected (0.024 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               1 |
+---------------------------------+---------------------------------+
1 row in set (0.108 sec)

Doris> set debug_skip_fold_constant=false;
Query OK, 0 rows affected (0.002 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               0 |
+---------------------------------+---------------------------------+

now:

Doris> set debug_skip_fold_constant=true;
Query OK, 0 rows affected (0.012 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               1 |
+---------------------------------+---------------------------------+
1 row in set (0.070 sec)

Doris> set debug_skip_fold_constant=false;
Query OK, 0 rows affected (0.002 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               1 |
+---------------------------------+---------------------------------+
1 row in set (0.010 sec)

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

@linrrzqqq
Copy link
Copy Markdown
Collaborator Author

/review

@linrrzqqq
Copy link
Copy Markdown
Collaborator Author

run buildall

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Reviewed the actual GitHub PR diff for PR #63954 (4 changed files). No blocking issues found.

Critical checkpoint conclusions:

  • Goal and tests: The change preserves the sign of negative zero in Nereids constant folding for SIGNBIT. The added unit test and regression test cover +0.0 and -0.0, including fold-constant coverage.
  • Scope: The actual PR diff is small and focused on SIGNBIT constant folding plus expected results.
  • Concurrency and lifecycle: Not applicable; this is pure constant-folding logic with no shared mutable state or lifecycle changes.
  • Configuration and compatibility: No config, storage format, wire protocol, or incompatible persistence changes.
  • Parallel paths: Runtime SQL behavior is already expected to preserve signbit semantics; this aligns FE constant folding with that behavior.
  • Special conditions: Using Double.doubleToRawLongBits checks the IEEE sign bit directly, which correctly distinguishes -0.0 from +0.0.
  • Test coverage: Existing additions cover FE executable function behavior and SQL regression output for constants. No additional user-provided review focus was supplied.
  • Observability, transactions, data writes, FE-BE variable passing, and memory tracking: Not applicable to this change.
  • Performance: No concern; the replacement is constant-time and only used during constant folding.

@hello-stephen
Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 100.00% (1/1) 🎉
Increment coverage report
Complete coverage report

Copy link
Copy Markdown
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jun 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

PR approved by anyone and no changes requested.

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.62% (1/161) 🎉
Increment coverage report
Complete coverage report

@hello-stephen
Copy link
Copy Markdown
Contributor

TPC-H: Total hot run time: 29315 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 9062a3dcc262d98b2271737ce6600715197f3f47, 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	17774	4077	4068	4068
q2	q3	10744	1393	803	803
q4	4701	476	337	337
q5	7530	870	591	591
q6	183	174	136	136
q7	777	858	631	631
q8	9361	1583	1573	1573
q9	5781	4492	4489	4489
q10	6751	1824	1548	1548
q11	436	270	247	247
q12	623	430	286	286
q13	18099	3381	2788	2788
q14	271	258	236	236
q15	q16	820	775	715	715
q17	966	989	1006	989
q18	6868	5857	5512	5512
q19	1278	1274	1116	1116
q20	529	412	257	257
q21	6282	2817	2685	2685
q22	457	382	308	308
Total cold run time: 100231 ms
Total hot run time: 29315 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	5273	4970	4986	4970
q2	q3	4952	5431	4624	4624
q4	2146	2192	1403	1403
q5	4843	4850	4730	4730
q6	238	180	129	129
q7	1842	1772	1589	1589
q8	2430	2104	2112	2104
q9	7856	7890	7445	7445
q10	4725	4696	4252	4252
q11	540	385	354	354
q12	723	744	523	523
q13	3049	3417	2825	2825
q14	273	279	253	253
q15	q16	663	696	604	604
q17	1293	1264	1248	1248
q18	7231	6845	6731	6731
q19	1107	1080	1094	1080
q20	2221	2205	1949	1949
q21	5302	4563	4448	4448
q22	507	455	393	393
Total cold run time: 57214 ms
Total hot run time: 51654 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

TPC-DS: Total hot run time: 170634 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 9062a3dcc262d98b2271737ce6600715197f3f47, data reload: false

query5	4332	657	519	519
query6	329	225	204	204
query7	4267	571	295	295
query8	321	228	227	227
query9	8794	4012	4006	4006
query10	454	354	299	299
query11	5789	2351	2219	2219
query12	189	142	129	129
query13	1267	646	455	455
query14	6095	5468	5137	5137
query14_1	4481	4446	4448	4446
query15	217	207	185	185
query16	1011	470	433	433
query17	1202	755	610	610
query18	2553	495	362	362
query19	229	209	175	175
query20	140	143	134	134
query21	221	138	118	118
query22	13641	13591	13296	13296
query23	17307	16488	16301	16301
query23_1	16427	16239	16289	16239
query24	7386	1749	1311	1311
query24_1	1319	1305	1333	1305
query25	562	500	419	419
query26	1312	312	177	177
query27	2744	566	351	351
query28	4377	1991	2006	1991
query29	977	614	491	491
query30	316	231	204	204
query31	1123	1075	936	936
query32	87	77	74	74
query33	534	363	292	292
query34	1185	1149	655	655
query35	784	784	701	701
query36	1423	1404	1262	1262
query37	164	102	86	86
query38	3215	3168	3037	3037
query39	920	931	893	893
query39_1	884	873	880	873
query40	235	147	128	128
query41	64	62	60	60
query42	120	110	110	110
query43	324	335	291	291
query44	
query45	213	202	196	196
query46	1071	1177	736	736
query47	2405	2426	2289	2289
query48	417	417	284	284
query49	646	498	387	387
query50	936	351	249	249
query51	4328	4308	4255	4255
query52	106	104	94	94
query53	252	288	207	207
query54	312	272	255	255
query55	94	98	84	84
query56	305	316	308	308
query57	1435	1430	1322	1322
query58	308	270	275	270
query59	1627	1655	1424	1424
query60	315	318	299	299
query61	164	147	157	147
query62	697	652	578	578
query63	245	197	213	197
query64	2468	787	636	636
query65	
query66	1701	481	348	348
query67	29782	29655	29525	29525
query68	
query69	461	341	304	304
query70	1072	1058	988	988
query71	311	279	275	275
query72	3103	2664	2341	2341
query73	854	733	433	433
query74	5099	4945	4754	4754
query75	2685	2623	2272	2272
query76	2290	1131	816	816
query77	403	398	339	339
query78	12397	12330	11860	11860
query79	1400	1022	777	777
query80	626	531	502	502
query81	451	277	248	248
query82	628	154	122	122
query83	356	272	248	248
query84	260	140	115	115
query85	866	532	453	453
query86	405	358	301	301
query87	3397	3370	3209	3209
query88	3648	2772	2740	2740
query89	439	393	345	345
query90	1933	182	188	182
query91	177	162	139	139
query92	76	73	75	73
query93	1469	1559	865	865
query94	553	345	301	301
query95	687	396	454	396
query96	1013	810	345	345
query97	2747	2749	2642	2642
query98	232	234	230	230
query99	1137	1162	1076	1076
Total cold run time: 253340 ms
Total hot run time: 170634 ms

@zclllyybb zclllyybb merged commit a2e76e0 into apache:master Jun 2, 2026
33 checks passed
github-actions Bot pushed a commit that referenced this pull request Jun 2, 2026
#63954)

Problem Summary:

`signbit` in Nereids FE constant folding used `value < 0` to determine
the sign, which treats `-0.0` as non-negative and folds it to `false`.
This is inconsistent with:
- the runtime BE implementation, which uses `std::signbit`
- the documented `signbit` behavior, which distinguishes `+0.0` and
`-0.0`

`+0.0` and `-0.0` compare equal numerically, so `value < 0` cannot
distinguish them. Their difference is only recorded in the
floating-point sign bit. Using raw bits makes FE constant folding
consistent with BE runtime semantics.

before:
```text
Doris> set debug_skip_fold_constant=true;
Query OK, 0 rows affected (0.024 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               1 |
+---------------------------------+---------------------------------+
1 row in set (0.108 sec)

Doris> set debug_skip_fold_constant=false;
Query OK, 0 rows affected (0.002 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               0 |
+---------------------------------+---------------------------------+
```

now:
```text
Doris> set debug_skip_fold_constant=true;
Query OK, 0 rows affected (0.012 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               1 |
+---------------------------------+---------------------------------+
1 row in set (0.070 sec)

Doris> set debug_skip_fold_constant=false;
Query OK, 0 rows affected (0.002 sec)

Doris> select signbit(cast('+0.0' as double)) , signbit(cast('-0.0' as double));
+---------------------------------+---------------------------------+
| signbit(cast('+0.0' as double)) | signbit(cast('-0.0' as double)) |
+---------------------------------+---------------------------------+
|                               0 |                               1 |
+---------------------------------+---------------------------------+
1 row in set (0.010 sec)
```
@linrrzqqq linrrzqqq deleted the fix-signbit-fold-constant branch June 2, 2026 02:00
yiguolei pushed a commit that referenced this pull request Jun 2, 2026
…stant folding #63954 (#63973)

Cherry-picked from #63954

Co-authored-by: linrrarity <linzhenqi@selectdb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.1.2-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants