Skip to content

Conversation

@morrySnow
Copy link
Contributor

What problem does this PR solve?

Problem Summary:

we use Literal#stringValue to generate decimal object. however, null literal could not return valid decimal string, so we should process it specially.

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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request addresses an issue where null literals are not correctly handled when generating decimal objects by introducing a dedicated check in the precision verification logic and adding a regression test.

  • Added a regression test for decimal precision computation in a Groovy suite.
  • Modified the decimal precision check in SearchSignature.java to account explicitly for null literals.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
regression-test/suites/nereids_syntax_p0/decimal_percision_compute.groovy Added a regression test suite for validating decimal precision with null literal inputs.
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/SearchSignature.java Updated the logic to handle null literals during decimal precision checks.
Comments suppressed due to low confidence (1)

regression-test/suites/nereids_syntax_p0/decimal_percision_compute.groovy:1

  • The file name contains a spelling mistake ('percision'). Consider renaming it to 'decimal_precision_compute.groovy' for clarity.
// Licensed to the Apache Software Foundation (ASF)...

@morrySnow
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TPC-H: Total hot run time: 33860 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 2579ca2830bfd6ebb75c5ffd03f67ecd7a5ddb62, data reload: false

------ Round 1 ----------------------------------
q1	26756	5015	5024	5015
q2	2072	299	193	193
q3	10523	1299	690	690
q4	10243	986	528	528
q5	7748	2384	2332	2332
q6	188	167	136	136
q7	915	763	622	622
q8	9334	1303	1077	1077
q9	6789	5160	5049	5049
q10	6860	2303	1874	1874
q11	497	285	267	267
q12	348	350	218	218
q13	17779	3700	3091	3091
q14	237	243	216	216
q15	524	490	504	490
q16	427	425	376	376
q17	629	868	385	385
q18	7657	7130	7102	7102
q19	1735	958	580	580
q20	337	344	222	222
q21	4062	2619	2426	2426
q22	1012	999	971	971
Total cold run time: 116672 ms
Total hot run time: 33860 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5175	5052	5041	5041
q2	235	322	226	226
q3	2173	2691	2304	2304
q4	1353	1753	1339	1339
q5	4500	4407	4384	4384
q6	217	169	136	136
q7	2011	1905	1759	1759
q8	2625	2587	2568	2568
q9	7209	7253	6934	6934
q10	3023	3209	2768	2768
q11	569	526	497	497
q12	705	789	619	619
q13	3563	3981	3293	3293
q14	284	314	301	301
q15	537	482	468	468
q16	446	485	437	437
q17	1146	1557	1394	1394
q18	7745	7509	7446	7446
q19	825	817	948	817
q20	1982	2048	1811	1811
q21	5050	4791	4738	4738
q22	1119	1069	1046	1046
Total cold run time: 52492 ms
Total hot run time: 50326 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 193113 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 2579ca2830bfd6ebb75c5ffd03f67ecd7a5ddb62, data reload: false

query1	1422	1083	1055	1055
query2	6162	1825	1856	1825
query3	10972	4537	4490	4490
query4	57882	25194	22903	22903
query5	5106	478	450	450
query6	366	201	190	190
query7	4957	525	290	290
query8	322	257	246	246
query9	6052	2678	2661	2661
query10	447	333	269	269
query11	15070	15462	14855	14855
query12	162	114	110	110
query13	1090	537	426	426
query14	10186	6365	6312	6312
query15	214	196	183	183
query16	7192	654	423	423
query17	1097	751	569	569
query18	1710	406	316	316
query19	202	189	177	177
query20	125	130	118	118
query21	206	128	107	107
query22	4513	4461	4323	4323
query23	34485	33387	33492	33387
query24	6551	2486	2459	2459
query25	476	478	401	401
query26	697	276	160	160
query27	2433	519	351	351
query28	3083	2182	2179	2179
query29	593	571	442	442
query30	271	219	193	193
query31	899	871	752	752
query32	77	73	71	71
query33	464	377	311	311
query34	770	878	550	550
query35	797	846	762	762
query36	972	1023	894	894
query37	114	104	105	104
query38	4305	4298	4262	4262
query39	1489	1476	1468	1468
query40	204	122	110	110
query41	68	57	51	51
query42	125	116	112	112
query43	511	531	471	471
query44	1436	846	853	846
query45	185	178	168	168
query46	862	1033	666	666
query47	1880	1873	1801	1801
query48	417	460	336	336
query49	703	520	454	454
query50	709	714	401	401
query51	4298	4275	4277	4275
query52	120	112	105	105
query53	237	265	199	199
query54	616	595	551	551
query55	93	90	89	89
query56	370	327	297	297
query57	1157	1182	1145	1145
query58	285	268	272	268
query59	2709	2799	2664	2664
query60	364	347	332	332
query61	151	150	153	150
query62	734	764	669	669
query63	235	202	194	194
query64	1957	1060	677	677
query65	4255	4257	4247	4247
query66	702	396	308	308
query67	15699	15804	15528	15528
query68	8257	901	529	529
query69	547	321	271	271
query70	1235	1154	1080	1080
query71	486	318	296	296
query72	5852	4816	5090	4816
query73	1470	708	358	358
query74	8993	9358	8882	8882
query75	3847	3277	2721	2721
query76	4093	1204	758	758
query77	638	390	283	283
query78	10038	10173	9230	9230
query79	2214	813	587	587
query80	611	525	447	447
query81	466	259	227	227
query82	454	124	103	103
query83	254	249	235	235
query84	286	103	85	85
query85	885	345	311	311
query86	422	311	295	295
query87	4391	4490	4337	4337
query88	3907	2327	2320	2320
query89	406	321	295	295
query90	1802	218	214	214
query91	141	142	112	112
query92	80	62	54	54
query93	1865	955	584	584
query94	651	392	317	317
query95	369	293	287	287
query96	495	565	285	285
query97	3215	3266	3132	3132
query98	237	210	199	199
query99	1475	1393	1305	1305
Total cold run time: 305095 ms
Total hot run time: 193113 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 29.41 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 2579ca2830bfd6ebb75c5ffd03f67ecd7a5ddb62, data reload: false

query1	0.04	0.04	0.03
query2	0.12	0.10	0.11
query3	0.27	0.19	0.19
query4	1.59	0.19	0.11
query5	0.56	0.56	0.56
query6	1.18	0.73	0.72
query7	0.02	0.02	0.02
query8	0.04	0.03	0.04
query9	0.59	0.52	0.50
query10	0.55	0.57	0.56
query11	0.16	0.12	0.12
query12	0.16	0.11	0.11
query13	0.61	0.60	0.60
query14	0.78	0.80	0.80
query15	0.88	0.86	0.87
query16	0.38	0.39	0.40
query17	1.05	1.01	1.06
query18	0.23	0.22	0.21
query19	1.92	1.84	1.82
query20	0.01	0.02	0.01
query21	15.42	0.89	0.53
query22	0.78	1.22	0.72
query23	14.80	1.41	0.62
query24	7.10	1.01	0.87
query25	0.51	0.26	0.12
query26	0.64	0.16	0.14
query27	0.06	0.05	0.06
query28	8.94	0.96	0.45
query29	12.54	4.04	3.38
query30	0.25	0.09	0.07
query31	2.81	0.64	0.38
query32	3.23	0.57	0.49
query33	3.01	3.15	3.04
query34	15.84	5.30	4.50
query35	4.48	4.51	4.50
query36	0.69	0.50	0.48
query37	0.09	0.07	0.07
query38	0.04	0.04	0.04
query39	0.04	0.03	0.02
query40	0.18	0.13	0.12
query41	0.08	0.03	0.02
query42	0.03	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 102.73 s
Total hot run time: 29.41 s

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label May 13, 2025
@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@morrySnow morrySnow merged commit 66ef4ef into apache:master May 14, 2025
28 of 30 checks passed
github-actions bot pushed a commit that referenced this pull request May 14, 2025
…ral (#50815)

### What problem does this PR solve?

we use Literal#stringValue to generate decimal object. however, null
literal could not return valid decimal string, so we should process it
specially.
github-actions bot pushed a commit that referenced this pull request May 14, 2025
…ral (#50815)

### What problem does this PR solve?

we use Literal#stringValue to generate decimal object. however, null
literal could not return valid decimal string, so we should process it
specially.
dataroaring pushed a commit that referenced this pull request May 17, 2025
…or null literal #50815 (#50898)

Cherry-picked from #50815

Co-authored-by: morrySnow <zhangwenxin@selectdb.com>
yiguolei pushed a commit that referenced this pull request May 17, 2025
…ral (#50815)

### What problem does this PR solve?

we use Literal#stringValue to generate decimal object. however, null
literal could not return valid decimal string, so we should process it
specially.
yiguolei pushed a commit that referenced this pull request May 22, 2025
…or null literal #50815 (#50899)

Cherry-picked from #50815

Co-authored-by: morrySnow <zhangwenxin@selectdb.com>
koarz pushed a commit to koarz/doris that referenced this pull request Jun 4, 2025
…ral (apache#50815)

### What problem does this PR solve?

we use Literal#stringValue to generate decimal object. however, null
literal could not return valid decimal string, so we should process it
specially.
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/2.1.11-merged dev/3.0.6-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants