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.

Sign up for GitHub

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

Jump to bottom

[fix](nereids) fix comparison with date like #45735

Merged
merged 1 commit into from
Dec 23, 2024
Merged

[fix](nereids) fix comparison with date like #45735

merged 1 commit into from
Dec 23, 2024

Conversation

yujun777
Copy link
Collaborator

@yujun777 yujun777 commented Dec 20, 2024 •
edited by morrySnow
Loading

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #45382

Problem Summary:

#45382 had fix compare date/datev1 with datetime literal wrong cutting. but it not fix completely.

if (right instanceof DateTimeLiteral) {
                    DateTimeLiteral dateTimeLiteral = (DateTimeLiteral) right;
                    right = migrateToDateV2(dateTimeLiteral);
                    if (dateTimeLiteral.getHour() != 0 || dateTimeLiteral.getMinute() != 0
                            || dateTimeLiteral.getSecond() != 0) {
                            ...
                    }
}

For the above code, if check right is date time literal, but notice that datetimev2 literal is datetime literal's child class. so datetimev2 literal will also run the above code. And datetimev2 literal should check its microseconds not equals to 0.

for example: date_a = '2020-01-01 00:00:00.01' should opt as FALSE, but not date_a = '2020-01-01'.

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
Copy link
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?

@yujun777
Copy link
Collaborator Author

run buildall

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 20, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17627	7504	7298	7298
q2	2059	182	184	182
q3	10528	1163	1128	1128
q4	10218	770	795	770
q5	7613	2730	2673	2673
q6	240	157	150	150
q7	1011	640	628	628
q8	9260	1947	1963	1947
q9	6650	6467	6473	6467
q10	7027	2304	2326	2304
q11	476	265	269	265
q12	433	225	221	221
q13	17779	2902	2961	2902
q14	237	211	210	210
q15	566	513	509	509
q16	670	600	599	599
q17	1007	587	526	526
q18	7368	6754	6764	6754
q19	1341	973	1032	973
q20	471	183	184	183
q21	3996	3344	3440	3344
q22	385	321	323	321
Total cold run time: 106962 ms
Total hot run time: 40354 ms

----- Round 2, with runtime_filter_mode=off -----
q1	8188	7214	7247	7214
q2	328	224	227	224
q3	2895	2946	3108	2946
q4	2132	1856	1812	1812
q5	5635	5715	5694	5694
q6	225	141	147	141
q7	2214	1753	1840	1753
q8	3380	3546	3515	3515
q9	8976	9000	8971	8971
q10	3591	3564	3546	3546
q11	593	524	497	497
q12	820	591	625	591
q13	13155	3131	2962	2962
q14	305	291	280	280
q15	558	497	495	495
q16	673	620	665	620
q17	1834	1613	1580	1580
q18	7871	7556	7518	7518
q19	1726	1447	1371	1371
q20	2067	1814	1838	1814
q21	5504	5315	5302	5302
q22	623	590	570	570
Total cold run time: 73293 ms
Total hot run time: 59416 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 190109 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 81a9495472f7182aadfbb73e61810f5e35dcbb70, data reload: false

query1	967	384	400	384
query2	6532	2380	2333	2333
query3	6712	210	219	210
query4	33494	23299	23394	23299
query5	4360	474	470	470
query6	300	207	203	203
query7	4643	311	322	311
query8	307	246	223	223
query9	9408	2746	2731	2731
query10	473	249	266	249
query11	17858	15283	15166	15166
query12	164	103	104	103
query13	1660	423	410	410
query14	11498	6755	7275	6755
query15	312	185	192	185
query16	8214	437	438	437
query17	1805	593	566	566
query18	2158	325	313	313
query19	373	163	165	163
query20	122	110	112	110
query21	215	102	108	102
query22	4513	4376	4443	4376
query23	34561	33581	37751	33581
query24	11938	2459	2516	2459
query25	565	410	413	410
query26	1101	153	156	153
query27	2773	331	328	328
query28	7649	2450	2440	2440
query29	657	402	415	402
query30	300	149	166	149
query31	1055	803	817	803
query32	93	56	57	56
query33	789	296	296	296
query34	985	523	501	501
query35	869	745	734	734
query36	1105	927	921	921
query37	208	77	121	77
query38	4251	4290	4186	4186
query39	1524	1427	1419	1419
query40	288	104	103	103
query41	48	46	45	45
query42	121	99	104	99
query43	531	489	489	489
query44	1264	804	795	795
query45	189	163	170	163
query46	1130	703	742	703
query47	1952	1786	1821	1786
query48	404	329	328	328
query49	1223	413	379	379
query50	832	389	375	375
query51	7287	7019	7102	7019
query52	101	96	90	90
query53	257	197	186	186
query54	1124	403	407	403
query55	82	78	83	78
query56	257	236	233	233
query57	1248	1112	1116	1112
query58	232	249	220	220
query59	3205	2993	2823	2823
query60	276	240	252	240
query61	138	112	118	112
query62	861	677	689	677
query63	220	193	186	186
query64	5034	677	660	660
query65	3257	3221	3237	3221
query66	1077	315	331	315
query67	15864	15546	15531	15531
query68	5960	544	545	544
query69	426	254	294	254
query70	1093	1145	1136	1136
query71	429	260	252	252
query72	6339	4033	4130	4033
query73	796	366	364	364
query74	10026	9010	8917	8917
query75	3449	2605	2654	2605
query76	3586	1088	1161	1088
query77	553	274	289	274
query78	10218	9351	9380	9351
query79	2492	596	611	596
query80	1111	456	444	444
query81	534	231	237	231
query82	755	118	122	118
query83	244	149	151	149
query84	234	71	71	71
query85	1644	331	305	305
query86	496	314	293	293
query87	4715	4375	4279	4279
query88	3924	2220	2207	2207
query89	416	294	306	294
query90	2094	201	193	193
query91	144	108	106	106
query92	66	52	53	52
query93	1788	547	558	547
query94	884	288	289	288
query95	365	255	257	255
query96	629	285	282	282
query97	2847	2692	2690	2690
query98	218	209	196	196
query99	1569	1310	1326	1310
Total cold run time: 305057 ms
Total hot run time: 190109 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.08	0.03	0.03
query3	0.23	0.07	0.07
query4	1.61	0.10	0.10
query5	0.43	0.40	0.42
query6	1.16	0.66	0.66
query7	0.02	0.02	0.01
query8	0.04	0.04	0.03
query9	0.59	0.51	0.50
query10	0.55	0.57	0.56
query11	0.15	0.10	0.10
query12	0.14	0.11	0.12
query13	0.61	0.61	0.61
query14	2.86	2.88	2.84
query15	0.90	0.83	0.82
query16	0.38	0.39	0.39
query17	0.99	1.04	1.03
query18	0.22	0.22	0.21
query19	1.88	1.79	2.02
query20	0.01	0.01	0.01
query21	15.36	0.59	0.60
query22	2.71	2.16	1.75
query23	17.00	0.86	0.74
query24	3.59	1.16	0.75
query25	0.25	0.18	0.14
query26	0.33	0.14	0.15
query27	0.04	0.05	0.04
query28	10.27	1.10	1.06
query29	12.60	3.22	3.19
query30	0.25	0.06	0.06
query31	2.85	0.40	0.39
query32	3.22	0.46	0.45
query33	3.05	3.07	3.08
query34	17.15	4.49	4.51
query35	4.46	4.42	4.46
query36	0.68	0.48	0.48
query37	0.09	0.06	0.06
query38	0.05	0.04	0.04
query39	0.03	0.02	0.02
query40	0.16	0.12	0.12
query41	0.09	0.03	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 107.19 s
Total hot run time: 32.37 s

@yujun777
Copy link
Collaborator Author

run p0

@englefly englefly merged commit c2e048f into apache:master Dec 23, 2024
33 of 34 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 23, 2024
@yujun777
[fix](nereids) fix comparison with date like (#45735)
e576fd9 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #45382

Problem Summary:

#45382 had fix compare date/datev1 with datetime literal wrong cutting.
but it not fix completely.

```
if (right instanceof DateTimeLiteral) {
                    DateTimeLiteral dateTimeLiteral = (DateTimeLiteral) right;
                    right = migrateToDateV2(dateTimeLiteral);
                    if (dateTimeLiteral.getHour() != 0 || dateTimeLiteral.getMinute() != 0
                            || dateTimeLiteral.getSecond() != 0) {
                            ...
                    }
}
```


For the above code, if check right is date time literal, but notice that
datetimev2 literal is datetime literal's child class. so datetimev2
literal will also run the above code. And datetimev2 literal should
check its microseconds not equals to 0.

for example: `date_a = '2020-01-01 00:00:00.01'` should opt as `FALSE`,
but not `date_a = '2020-01-01'`.
@github-actions github-actions bot mentioned this pull request Dec 23, 2024
Merged
github-actions bot pushed a commit that referenced this pull request Dec 23, 2024
@yujun777
[fix](nereids) fix comparison with date like (#45735)
3a90d30 
### What problem does this PR solve?

Issue Number: close #xxx

Related PR: #45382

Problem Summary:

#45382 had fix compare date/datev1 with datetime literal wrong cutting.
but it not fix completely.

```
if (right instanceof DateTimeLiteral) {
                    DateTimeLiteral dateTimeLiteral = (DateTimeLiteral) right;
                    right = migrateToDateV2(dateTimeLiteral);
                    if (dateTimeLiteral.getHour() != 0 || dateTimeLiteral.getMinute() != 0
                            || dateTimeLiteral.getSecond() != 0) {
                            ...
                    }
}
```


For the above code, if check right is date time literal, but notice that
datetimev2 literal is datetime literal's child class. so datetimev2
literal will also run the above code. And datetimev2 literal should
check its microseconds not equals to 0.

for example: `date_a = '2020-01-01 00:00:00.01'` should opt as `FALSE`,
but not `date_a = '2020-01-01'`.
@github-actions github-actions bot mentioned this pull request Dec 23, 2024
Merged
englefly pushed a commit that referenced this pull request Dec 23, 2024
@github-actions @yujun777
branch-2.1: [fix](nereids) fix comparison with date like #45735 (#45781)
412355b 
Cherry-picked from #45735

Co-authored-by: yujun <yujun@selectdb.com>
englefly pushed a commit that referenced this pull request Dec 23, 2024
@github-actions @yujun777
branch-3.0: [fix](nereids) fix comparison with date like #45735 (#45779)
8e38de9 
Cherry-picked from #45735

Co-authored-by: yujun <yujun@selectdb.com>
@yiguolei yiguolei mentioned this pull request Jan 19, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@englefly englefly englefly approved these changes

@morrySnow morrySnow morrySnow approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.8-merged dev/3.0.4-merged p0_b reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yujun777 @hello-stephen @doris-robot @englefly @morrySnow