Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BugFix](Variant) limit cast elimination to one level cast #47778

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

eldenmoon
Copy link
Member

The expression like cast(cast(v['a'] as bigint) as double) in (1.1, 1.2) which contains 2 levels cast expr should not push down.Otherwise leading to incorrect result or crash

What problem does this PR solve?

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

@Thearas
Copy link
Contributor

Thearas commented Feb 11, 2025

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?

The expression like `cast(cast(v['a'] as bigint) as double) in (1.1, 1.2)` which contains 2 levels cast expr should not push down.Otherwise leading to incorrect result or crash
@eldenmoon
Copy link
Member Author

run buildall

1 similar comment
@eldenmoon
Copy link
Member Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 42.81% (11262/26308)
Line Coverage: 32.79% (94620/288585)
Region Coverage: 31.95% (48506/151835)
Branch Coverage: 27.81% (24457/87950)
Coverage Report: http://coverage.selectdb-in.cc/coverage/5bb393dbfa6585e8b71fb08d383838e6528724f6_5bb393dbfa6585e8b71fb08d383838e6528724f6/report/index.html

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 42.81% (11262/26309)
Line Coverage: 32.78% (94593/288591)
Region Coverage: 31.94% (48499/151833)
Branch Coverage: 27.81% (24461/87950)
Coverage Report: http://coverage.selectdb-in.cc/coverage/5bb393dbfa6585e8b71fb08d383838e6528724f6_5bb393dbfa6585e8b71fb08d383838e6528724f6/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17593	5229	5113	5113
q2	2039	291	167	167
q3	10418	1240	747	747
q4	10252	997	531	531
q5	7565	2486	2372	2372
q6	190	170	137	137
q7	906	762	632	632
q8	9316	1318	1151	1151
q9	4855	4727	4693	4693
q10	6888	2291	1908	1908
q11	491	277	249	249
q12	349	358	224	224
q13	17756	3683	3122	3122
q14	234	230	214	214
q15	505	470	463	463
q16	620	601	580	580
q17	572	866	346	346
q18	6514	6302	6270	6270
q19	2562	970	560	560
q20	297	309	185	185
q21	2665	2176	1961	1961
q22	360	323	295	295
Total cold run time: 102947 ms
Total hot run time: 31920 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5317	5157	5575	5157
q2	234	328	244	244
q3	2135	2669	2259	2259
q4	1410	1799	1384	1384
q5	4211	4117	4111	4111
q6	209	170	124	124
q7	1854	1820	1789	1789
q8	2647	2549	2593	2549
q9	7201	7112	7214	7112
q10	3021	3221	2788	2788
q11	586	508	479	479
q12	701	786	628	628
q13	3480	3944	3325	3325
q14	273	308	262	262
q15	496	474	468	468
q16	657	672	635	635
q17	1122	1594	1374	1374
q18	7479	7254	7318	7254
q19	848	868	1010	868
q20	1961	2009	1895	1895
q21	5449	5121	4605	4605
q22	604	569	551	551
Total cold run time: 51895 ms
Total hot run time: 49861 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 189527 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 5bb393dbfa6585e8b71fb08d383838e6528724f6, data reload: false

query1	1304	937	954	937
query2	6141	1811	1819	1811
query3	11061	4681	4681	4681
query4	54906	25248	22989	22989
query5	5069	564	473	473
query6	338	194	192	192
query7	4917	515	291	291
query8	314	247	233	233
query9	5653	2508	2494	2494
query10	431	315	264	264
query11	15169	14986	14999	14986
query12	164	108	108	108
query13	1077	528	390	390
query14	10526	6306	6284	6284
query15	203	204	192	192
query16	7084	663	480	480
query17	1083	736	612	612
query18	1517	448	332	332
query19	203	215	178	178
query20	131	127	118	118
query21	205	123	108	108
query22	4357	4602	4335	4335
query23	33903	33240	33352	33240
query24	5925	2400	2452	2400
query25	467	470	418	418
query26	677	281	164	164
query27	1910	474	330	330
query28	2802	2426	2404	2404
query29	576	541	452	452
query30	231	193	159	159
query31	904	894	871	871
query32	72	65	57	57
query33	438	344	319	319
query34	770	859	525	525
query35	809	833	748	748
query36	964	994	892	892
query37	126	100	78	78
query38	4361	4336	4233	4233
query39	1522	1450	1419	1419
query40	204	118	105	105
query41	57	60	57	57
query42	118	121	108	108
query43	523	509	465	465
query44	1327	802	809	802
query45	181	171	165	165
query46	881	1074	656	656
query47	1851	1867	1773	1773
query48	381	435	330	330
query49	676	518	417	417
query50	711	756	407	407
query51	4227	4257	4292	4257
query52	111	110	101	101
query53	250	276	191	191
query54	492	480	409	409
query55	83	84	87	84
query56	283	269	261	261
query57	1138	1176	1102	1102
query58	254	252	248	248
query59	2706	2778	2705	2705
query60	281	288	267	267
query61	114	116	115	115
query62	751	772	676	676
query63	227	192	197	192
query64	1428	1019	662	662
query65	3250	3099	3165	3099
query66	717	384	288	288
query67	15811	15463	15392	15392
query68	4871	791	519	519
query69	515	302	267	267
query70	1250	1154	1133	1133
query71	458	296	256	256
query72	6360	3638	3733	3638
query73	848	768	348	348
query74	9107	9004	8826	8826
query75	3181	3156	2671	2671
query76	3732	1175	790	790
query77	553	371	280	280
query78	10000	10087	9292	9292
query79	2598	846	598	598
query80	636	516	454	454
query81	487	279	244	244
query82	222	153	121	121
query83	169	168	153	153
query84	296	88	76	76
query85	740	339	294	294
query86	348	295	274	274
query87	4524	4542	4358	4358
query88	3574	2208	2191	2191
query89	393	312	278	278
query90	1689	195	191	191
query91	128	140	113	113
query92	74	59	54	54
query93	2190	1003	578	578
query94	678	409	301	301
query95	339	262	259	259
query96	479	567	265	265
query97	2826	2856	2716	2716
query98	234	207	205	205
query99	1306	1376	1253	1253
Total cold run time: 292148 ms
Total hot run time: 189527 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.07	0.04	0.02
query3	0.24	0.07	0.06
query4	1.62	0.10	0.10
query5	0.41	0.40	0.39
query6	1.16	0.66	0.67
query7	0.02	0.01	0.02
query8	0.04	0.03	0.03
query9	0.60	0.51	0.51
query10	0.58	0.60	0.56
query11	0.14	0.10	0.10
query12	0.14	0.11	0.11
query13	0.62	0.59	0.61
query14	2.79	2.73	2.82
query15	0.92	0.86	0.86
query16	0.37	0.36	0.36
query17	1.05	1.01	1.03
query18	0.20	0.20	0.19
query19	1.87	1.75	1.99
query20	0.02	0.01	0.02
query21	15.35	0.90	0.54
query22	0.74	1.03	0.67
query23	15.12	1.39	0.63
query24	7.52	1.18	0.88
query25	0.51	0.35	0.18
query26	0.65	0.16	0.14
query27	0.05	0.06	0.05
query28	9.82	0.87	0.43
query29	12.54	4.10	3.38
query30	0.25	0.09	0.06
query31	2.82	0.58	0.38
query32	3.22	0.55	0.48
query33	2.97	3.02	3.02
query34	15.77	5.11	4.55
query35	4.63	4.57	4.57
query36	0.66	0.48	0.48
query37	0.08	0.06	0.06
query38	0.04	0.04	0.04
query39	0.03	0.02	0.02
query40	0.16	0.13	0.12
query41	0.08	0.02	0.02
query42	0.04	0.02	0.03
query43	0.04	0.03	0.02
Total cold run time: 105.98 s
Total hot run time: 30.82 s

Copy link
Contributor

@csun5285 csun5285 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

PR approved by anyone and no changes requested.

Copy link
Contributor

@qidaye qidaye left a comment

Choose a reason for hiding this comment

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

LGTM

@eldenmoon eldenmoon merged commit 74c419c into apache:master Feb 17, 2025
25 of 26 checks passed
@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Feb 17, 2025
Copy link
Contributor

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

github-actions bot pushed a commit that referenced this pull request Feb 17, 2025
The expression like `cast(cast(v['a'] as bigint) as double) in (1.1,
1.2)` which contains 2 levels cast expr should not push down.Otherwise
leading to incorrect result or crash
github-actions bot pushed a commit that referenced this pull request Feb 17, 2025
The expression like `cast(cast(v['a'] as bigint) as double) in (1.1,
1.2)` which contains 2 levels cast expr should not push down.Otherwise
leading to incorrect result or crash
yiguolei pushed a commit that referenced this pull request Feb 19, 2025
…#47778 (#47955)

Cherry-picked from #47778

Co-authored-by: lihangyu <lihangyu@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/2.1.9-merged dev/3.0.x p0_w reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants