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

[fix](jdbc scan) Remove the conjuncts.remove call in JdbcScan #39180

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

zy-kkk
Copy link
Member

@zy-kkk zy-kkk commented Aug 9, 2024

In #37565, due to the change in the calling order of finalize, the final generated Plan will be missing the PREDICATES that have been pushed down in Jdbc. Although this behavior is correct, before perfectly handling the push down of various PREDICATES, we need to keep all conjuncts to ensure that we can still filter data normally when the data returned by Jdbc is a superset.

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@zy-kkk
Copy link
Member Author

zy-kkk commented Aug 9, 2024

run buildall

@github-actions github-actions bot added the doing label Aug 9, 2024
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17633	4347	4255	4255
q2	2023	175	197	175
q3	10480	1214	1130	1130
q4	10655	811	731	731
q5	7732	2566	2519	2519
q6	228	142	141	141
q7	977	605	607	605
q8	9319	1948	1955	1948
q9	8994	6654	6609	6609
q10	7036	2180	2239	2180
q11	453	243	247	243
q12	398	229	217	217
q13	18885	2968	2945	2945
q14	278	243	240	240
q15	539	494	483	483
q16	517	377	376	376
q17	966	635	709	635
q18	8181	7457	7453	7453
q19	3863	1048	1098	1048
q20	658	322	329	322
q21	5436	4462	4511	4462
q22	1104	1024	1015	1015
Total cold run time: 116355 ms
Total hot run time: 39732 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4435	4254	4284	4254
q2	386	265	263	263
q3	2841	2691	2638	2638
q4	1902	1687	1652	1652
q5	5242	5235	5206	5206
q6	217	131	129	129
q7	2030	1675	1684	1675
q8	3197	3332	3351	3332
q9	8381	8388	8361	8361
q10	3390	3119	3137	3119
q11	589	490	501	490
q12	791	591	590	590
q13	16727	2976	2968	2968
q14	308	269	274	269
q15	533	485	469	469
q16	484	428	408	408
q17	1775	1491	1469	1469
q18	7716	7566	7586	7566
q19	1698	1559	1528	1528
q20	2002	1785	1780	1780
q21	5449	5170	5117	5117
q22	1101	983	1022	983
Total cold run time: 71194 ms
Total hot run time: 54266 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 201033 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 823e6a6b96d54d62d9d78c92c520b823b6d5242d, data reload: false

query1	919	369	355	355
query2	6462	1835	1804	1804
query3	6648	211	223	211
query4	30709	23260	23106	23106
query5	4186	494	493	493
query6	289	186	177	177
query7	4604	293	291	291
query8	250	200	191	191
query9	8627	2452	2438	2438
query10	548	453	440	440
query11	17834	15044	14895	14895
query12	142	96	92	92
query13	1625	386	356	356
query14	9537	7550	7731	7550
query15	263	201	210	201
query16	7748	447	490	447
query17	1727	558	544	544
query18	1974	282	279	279
query19	197	140	139	139
query20	114	109	107	107
query21	203	102	100	100
query22	4470	4049	4106	4049
query23	33948	33415	33167	33167
query24	11936	2620	2599	2599
query25	616	359	374	359
query26	1695	159	153	153
query27	2848	274	277	274
query28	7695	2013	2000	2000
query29	1053	414	402	402
query30	303	153	155	153
query31	1020	752	758	752
query32	97	53	52	52
query33	749	315	286	286
query34	910	460	473	460
query35	941	830	824	824
query36	1079	912	886	886
query37	146	78	81	78
query38	4245	4112	4209	4112
query39	1417	1361	1357	1357
query40	267	118	113	113
query41	46	45	43	43
query42	116	94	96	94
query43	509	468	468	468
query44	1202	731	739	731
query45	233	205	199	199
query46	1079	746	762	746
query47	1852	1758	1783	1758
query48	370	296	303	296
query49	1143	424	407	407
query50	804	412	410	410
query51	6859	6633	6753	6633
query52	101	88	92	88
query53	257	189	177	177
query54	1049	457	443	443
query55	77	77	75	75
query56	259	245	237	237
query57	1172	1050	1041	1041
query58	256	219	227	219
query59	2992	2757	2729	2729
query60	287	273	265	265
query61	98	103	100	100
query62	845	649	646	646
query63	217	185	182	182
query64	10568	2288	1737	1737
query65	3229	3112	3150	3112
query66	1377	361	334	334
query67	15447	14759	14920	14759
query68	4519	543	548	543
query69	459	397	396	396
query70	1202	1115	1129	1115
query71	398	269	292	269
query72	17215	16896	16653	16653
query73	764	334	333	333
query74	9040	8757	8813	8757
query75	3380	2691	2692	2691
query76	2719	1028	1002	1002
query77	508	314	334	314
query78	9747	9197	8934	8934
query79	2299	525	523	523
query80	1897	524	515	515
query81	623	223	220	220
query82	955	141	137	137
query83	299	147	147	147
query84	265	88	81	81
query85	1409	341	330	330
query86	425	299	300	299
query87	4663	4521	4658	4521
query88	4119	2506	2509	2506
query89	394	289	293	289
query90	1781	197	196	196
query91	133	111	108	108
query92	66	53	51	51
query93	2161	541	531	531
query94	850	298	305	298
query95	362	270	275	270
query96	600	276	277	276
query97	3215	3083	3051	3051
query98	222	215	196	196
query99	1471	1320	1260	1260
Total cold run time: 310645 ms
Total hot run time: 201033 ms

@doris-robot
Copy link

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

query1	0.05	0.05	0.04
query2	0.08	0.04	0.04
query3	0.23	0.06	0.06
query4	1.66	0.09	0.10
query5	0.50	0.49	0.47
query6	1.13	0.74	0.73
query7	0.02	0.01	0.02
query8	0.04	0.04	0.04
query9	0.54	0.48	0.48
query10	0.54	0.55	0.54
query11	0.16	0.11	0.12
query12	0.15	0.12	0.12
query13	0.59	0.60	0.59
query14	0.76	0.79	0.77
query15	0.86	0.81	0.81
query16	0.36	0.36	0.36
query17	1.01	1.01	1.05
query18	0.22	0.22	0.22
query19	1.80	1.71	1.70
query20	0.01	0.01	0.01
query21	15.41	0.77	0.67
query22	3.57	7.41	2.26
query23	18.26	1.31	1.23
query24	2.05	0.22	0.23
query25	0.15	0.07	0.08
query26	0.30	0.21	0.21
query27	0.45	0.22	0.22
query28	13.38	1.01	1.01
query29	12.59	3.30	3.31
query30	0.23	0.05	0.05
query31	2.91	0.41	0.39
query32	3.25	0.48	0.46
query33	2.92	2.96	2.92
query34	17.08	4.30	4.37
query35	4.41	4.41	4.39
query36	0.64	0.48	0.49
query37	0.19	0.15	0.15
query38	0.15	0.14	0.14
query39	0.04	0.04	0.04
query40	0.15	0.12	0.13
query41	0.09	0.06	0.05
query42	0.05	0.04	0.04
query43	0.05	0.04	0.05
Total cold run time: 109.03 s
Total hot run time: 30.96 s

morningman
morningman previously approved these changes Aug 11, 2024
Copy link
Contributor

@morningman morningman 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 Aug 11, 2024
Copy link
Contributor

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

Copy link
Contributor

PR approved by anyone and no changes requested.

kaka11chen
kaka11chen previously approved these changes Aug 11, 2024
Copy link
Contributor

@kaka11chen kaka11chen left a comment

Choose a reason for hiding this comment

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

LGTM

@zy-kkk zy-kkk dismissed stale reviews from kaka11chen and morningman via bd01d8d August 11, 2024 13:23
@zy-kkk zy-kkk force-pushed the fix_jdbc_conjuncts branch from 823e6a6 to bd01d8d Compare August 11, 2024 13:23
@zy-kkk
Copy link
Member Author

zy-kkk commented Aug 11, 2024

run buildall

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Aug 11, 2024
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17602	4747	4282	4282
q2	2010	180	170	170
q3	10481	1195	1161	1161
q4	10146	755	681	681
q5	7493	2523	2538	2523
q6	226	141	141	141
q7	962	591	591	591
q8	9219	2029	1884	1884
q9	8632	6552	6521	6521
q10	7046	2232	2161	2161
q11	471	244	254	244
q12	464	226	221	221
q13	18830	2958	2989	2958
q14	294	236	240	236
q15	522	481	507	481
q16	502	390	396	390
q17	956	773	659	659
q18	8033	7411	7389	7389
q19	4869	1048	1107	1048
q20	690	338	330	330
q21	5269	4194	4459	4194
q22	1110	1015	989	989
Total cold run time: 115827 ms
Total hot run time: 39254 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4433	4260	4294	4260
q2	365	268	279	268
q3	2786	2646	2701	2646
q4	2001	1691	1694	1691
q5	5642	5538	5559	5538
q6	237	132	133	132
q7	2188	1728	1858	1728
q8	3291	3466	3421	3421
q9	8768	8766	8904	8766
q10	3536	3354	3287	3287
q11	588	500	493	493
q12	776	608	628	608
q13	17033	3169	3207	3169
q14	330	295	285	285
q15	537	491	503	491
q16	510	435	453	435
q17	1827	1519	1508	1508
q18	8023	8035	7916	7916
q19	1811	1522	1521	1521
q20	2205	1920	1867	1867
q21	10078	5388	5299	5299
q22	1100	1019	1064	1019
Total cold run time: 78065 ms
Total hot run time: 56348 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 201880 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 bd01d8d4c36dd9ee0cef9a56bd772bb084d0aa31, data reload: false

query1	902	374	358	358
query2	6481	2011	1914	1914
query3	6641	207	226	207
query4	34322	23363	23042	23042
query5	3643	479	511	479
query6	264	172	157	157
query7	4573	294	292	292
query8	230	209	203	203
query9	8505	2360	2339	2339
query10	524	489	439	439
query11	15539	14963	14934	14934
query12	132	96	97	96
query13	1637	381	366	366
query14	10234	7748	7817	7748
query15	277	217	228	217
query16	7061	528	507	507
query17	1380	582	589	582
query18	1785	298	302	298
query19	201	147	148	147
query20	109	108	110	108
query21	210	109	106	106
query22	4457	4202	4260	4202
query23	34618	33464	34207	33464
query24	8622	2727	2675	2675
query25	576	383	365	365
query26	707	160	160	160
query27	2143	293	291	291
query28	5871	1974	1967	1967
query29	717	397	408	397
query30	268	150	143	143
query31	989	764	738	738
query32	91	51	53	51
query33	603	282	276	276
query34	865	461	474	461
query35	923	849	844	844
query36	1073	922	924	922
query37	132	75	83	75
query38	4121	4213	4125	4125
query39	1450	1388	1391	1388
query40	186	115	113	113
query41	46	43	43	43
query42	116	98	93	93
query43	523	483	490	483
query44	1047	736	738	736
query45	266	207	208	207
query46	1075	747	719	719
query47	1834	1757	1756	1756
query48	373	305	293	293
query49	831	404	413	404
query50	796	413	407	407
query51	6661	6696	6605	6605
query52	105	96	92	92
query53	251	180	186	180
query54	794	453	442	442
query55	77	74	74	74
query56	273	248	250	248
query57	1132	1064	1068	1064
query58	227	224	254	224
query59	3083	2820	2920	2820
query60	283	257	261	257
query61	95	95	93	93
query62	794	652	670	652
query63	207	184	178	178
query64	9169	2243	1758	1758
query65	3200	3145	3144	3144
query66	752	333	328	328
query67	15244	14851	14646	14646
query68	6811	542	547	542
query69	498	420	415	415
query70	1216	1126	1121	1121
query71	468	271	261	261
query72	18378	16737	17051	16737
query73	821	324	326	324
query74	9214	8854	8744	8744
query75	3824	2783	2705	2705
query76	4027	983	893	893
query77	703	298	309	298
query78	11934	9749	9020	9020
query79	4166	517	518	517
query80	1901	505	489	489
query81	601	225	220	220
query82	633	141	134	134
query83	238	149	148	148
query84	275	78	77	77
query85	890	272	316	272
query86	375	292	288	288
query87	4627	4565	4621	4565
query88	4831	2486	2515	2486
query89	416	287	281	281
query90	1834	196	192	192
query91	122	95	96	95
query92	64	50	51	50
query93	4313	532	535	532
query94	616	302	312	302
query95	349	261	265	261
query96	612	274	272	272
query97	3274	3039	3022	3022
query98	225	214	202	202
query99	1690	1273	1282	1273
Total cold run time: 312262 ms
Total hot run time: 201880 ms

@doris-robot
Copy link

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

query1	0.05	0.05	0.03
query2	0.08	0.04	0.03
query3	0.23	0.05	0.06
query4	1.67	0.08	0.09
query5	0.48	0.47	0.48
query6	1.14	0.73	0.73
query7	0.02	0.01	0.02
query8	0.05	0.04	0.04
query9	0.54	0.49	0.50
query10	0.52	0.55	0.55
query11	0.15	0.11	0.12
query12	0.15	0.12	0.12
query13	0.61	0.62	0.59
query14	0.76	0.78	0.79
query15	0.85	0.82	0.81
query16	0.34	0.37	0.34
query17	0.97	1.01	0.97
query18	0.24	0.24	0.24
query19	1.85	1.74	1.73
query20	0.01	0.00	0.01
query21	15.39	0.75	0.65
query22	4.13	6.25	2.53
query23	18.26	1.37	1.26
query24	2.07	0.22	0.21
query25	0.15	0.08	0.08
query26	0.29	0.21	0.20
query27	0.46	0.23	0.22
query28	13.36	1.03	1.01
query29	12.66	3.38	3.41
query30	0.24	0.05	0.05
query31	2.90	0.39	0.38
query32	3.28	0.48	0.47
query33	2.86	2.95	2.86
query34	16.99	4.40	4.36
query35	4.37	4.44	4.48
query36	0.64	0.49	0.48
query37	0.20	0.15	0.15
query38	0.15	0.14	0.14
query39	0.05	0.03	0.03
query40	0.16	0.12	0.13
query41	0.09	0.05	0.05
query42	0.06	0.04	0.05
query43	0.04	0.04	0.04
Total cold run time: 109.51 s
Total hot run time: 31.32 s

Copy link
Contributor

@wuwenchi wuwenchi 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 Aug 12, 2024
Copy link
Contributor

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

@morningman morningman merged commit 81438d5 into apache:master Aug 12, 2024
31 of 32 checks passed
wyxxxcat pushed a commit to wyxxxcat/doris that referenced this pull request Aug 14, 2024
…he#39180)

In apache#37565, due to the change in the calling order of finalize, the final
generated Plan will be missing the PREDICATES that have been pushed down
in Jdbc. Although this behavior is correct, before perfectly handling
the push down of various PREDICATES, we need to keep all conjuncts to
ensure that we can still filter data normally when the data returned by
Jdbc is a superset.
@zy-kkk zy-kkk deleted the fix_jdbc_conjuncts branch August 15, 2024 06:51
zy-kkk added a commit to zy-kkk/doris that referenced this pull request Aug 15, 2024
…he#39180)

In apache#37565, due to the change in the calling order of finalize, the final
generated Plan will be missing the PREDICATES that have been pushed down
in Jdbc. Although this behavior is correct, before perfectly handling
the push down of various PREDICATES, we need to keep all conjuncts to
ensure that we can still filter data normally when the data returned by
Jdbc is a superset.
zy-kkk added a commit that referenced this pull request Aug 16, 2024
…bcScan (#39407)

pick (#39180)

In #37565, due to the change in the calling order of finalize, the final
generated Plan will be missing the PREDICATES that have been pushed down
in Jdbc. Although this behavior is correct, before perfectly handling
the push down of various PREDICATES, we need to keep all conjuncts to
ensure that we can still filter data normally when the data returned by
Jdbc is a superset.
dataroaring pushed a commit that referenced this pull request Aug 17, 2024
In #37565, due to the change in the calling order of finalize, the final
generated Plan will be missing the PREDICATES that have been pushed down
in Jdbc. Although this behavior is correct, before perfectly handling
the push down of various PREDICATES, we need to keep all conjuncts to
ensure that we can still filter data normally when the data returned by
Jdbc is a superset.
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.6-merged dev/3.0.2-merged doing p0_b reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants