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](correctness) need to update flag of _has_null in ColumnNullable::filter_by_selector #40756

Closed
wants to merge 1 commit into from
Closed

[fix](correctness) need to update flag of _has_null in ColumnNullable::filter_by_selector #40756

wants to merge 1 commit into from

Conversation

mrhhsg
Copy link
Member

@mrhhsg mrhhsg commented Sep 12, 2024

Proposed changes

If the flag _need_update_has_null is false after ColumnNullable::filter_by_selector, has_null will return incorrect value.

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

@mrhhsg
Copy link
Member Author

mrhhsg commented Sep 12, 2024

run buildall

zclllyybb
zclllyybb approved these changes Sep 12, 2024
View reviewed changes
Copy link
Contributor

@zclllyybb zclllyybb 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
Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17611	7538	7272	7272
q2	2032	187	184	184
q3	10477	1543	1397	1397
q4	10160	1003	1063	1003
q5	7782	3296	3226	3226
q6	245	156	153	153
q7	1052	638	629	629
q8	9474	2065	2026	2026
q9	6825	6326	6288	6288
q10	7032	2575	2525	2525
q11	444	251	260	251
q12	415	227	230	227
q13	17765	3032	3035	3032
q14	292	263	254	254
q15	580	536	521	521
q16	517	426	439	426
q17	1000	959	951	951
q18	7404	6929	6758	6758
q19	1381	1247	1239	1239
q20	624	348	335	335
q21	3893	3556	3508	3508
q22	1068	1003	1013	1003
Total cold run time: 108073 ms
Total hot run time: 43208 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7240	7227	7207	7207
q2	344	243	228	228
q3	2902	2889	2941	2889
q4	1949	1929	1957	1929
q5	5502	5433	5467	5433
q6	231	144	145	144
q7	2022	1678	1680	1678
q8	3269	3369	3334	3334
q9	8457	8440	8442	8440
q10	3409	3469	3458	3458
q11	585	475	480	475
q12	809	600	574	574
q13	6837	3069	3033	3033
q14	295	265	269	265
q15	574	521	535	521
q16	485	444	452	444
q17	1773	1714	1709	1709
q18	7941	7612	7701	7612
q19	1737	1714	1706	1706
q20	2044	1804	1815	1804
q21	5660	5398	5454	5398
q22	1091	1007	986	986
Total cold run time: 65156 ms
Total hot run time: 59267 ms

@cambyzju
Copy link
Contributor

Why I test the regression test case on 2.1.6 and 2.0.13, both got the same result as the output in this pr.

Is there any other steps to reproduce the problem?

cambyzju
cambyzju reviewed Sep 12, 2024
View reviewed changes
regression-test/suites/correctness/test_column_nullable_cache.groovy


/*
The varchar column type of the view needs to be a specific number, not an asterisk
Copy link
Contributor

Choose a reason for hiding this comment

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

useless comments?

@doris-robot
Copy link

TPC-DS: Total hot run time: 195395 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 8adddc72d3fdbee0abb8f4d3f1d8b9b546b64c2d, data reload: false

query1	911	382	392	382
query2	6522	1820	1801	1801
query3	6664	212	220	212
query4	25850	24178	24004	24004
query5	4990	560	535	535
query6	289	186	178	178
query7	4585	309	314	309
query8	289	229	234	229
query9	8501	2580	2590	2580
query10	469	309	294	294
query11	16163	15672	15528	15528
query12	167	101	108	101
query13	1701	406	388	388
query14	11286	7335	7070	7070
query15	215	175	178	175
query16	7611	471	465	465
query17	1562	586	568	568
query18	2004	293	297	293
query19	198	154	152	152
query20	124	112	113	112
query21	214	108	106	106
query22	4340	4278	4285	4278
query23	34470	33633	33705	33633
query24	10005	3147	3117	3117
query25	683	421	424	421
query26	1400	163	157	157
query27	2857	285	286	285
query28	6881	2118	2097	2097
query29	1051	408	414	408
query30	293	161	167	161
query31	973	760	805	760
query32	108	55	56	55
query33	748	293	305	293
query34	904	476	506	476
query35	874	745	709	709
query36	1035	912	909	909
query37	172	86	81	81
query38	4093	3850	3906	3850
query39	1461	1441	1417	1417
query40	286	114	115	114
query41	49	47	48	47
query42	119	101	99	99
query43	482	432	447	432
query44	1231	814	789	789
query45	195	171	171	171
query46	1128	835	818	818
query47	1889	1763	1789	1763
query48	373	290	297	290
query49	1130	492	440	440
query50	920	440	440	440
query51	6922	7033	6903	6903
query52	97	88	88	88
query53	259	181	186	181
query54	832	463	467	463
query55	79	73	78	73
query56	285	261	269	261
query57	1224	1089	1085	1085
query58	257	314	240	240
query59	2763	2609	2589	2589
query60	317	270	274	270
query61	106	103	101	101
query62	932	681	691	681
query63	225	191	188	188
query64	5303	693	701	693
query65	3249	3194	3168	3168
query66	1391	299	297	297
query67	16082	15429	15457	15429
query68	3179	874	854	854
query69	440	328	326	326
query70	1136	1173	1153	1153
query71	358	349	345	345
query72	6098	3400	3346	3346
query73	599	593	593	593
query74	9323	8998	9207	8998
query75	3123	3017	2994	2994
query76	1944	868	851	851
query77	440	415	400	400
query78	9469	9335	9229	9229
query79	924	897	874	874
query80	855	858	836	836
query81	452	272	272	272
query82	270	264	267	264
query83	219	197	194	194
query84	245	111	112	111
query85	666	424	403	403
query86	323	306	285	285
query87	4349	4300	4330	4300
query88	4287	4201	4214	4201
query89	383	370	377	370
query90	1310	327	323	323
query91	125	125	195	125
query92	82	76	76	76
query93	1057	1089	1057	1057
query94	620	398	349	349
query95	454	439	425	425
query96	489	483	480	480
query97	3112	3115	3156	3115
query98	225	231	227	227
query99	1556	1337	1317	1317
Total cold run time: 278900 ms
Total hot run time: 195395 ms

@doris-robot
Copy link

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

query1	0.05	0.04	0.04
query2	0.07	0.04	0.04
query3	0.22	0.05	0.05
query4	1.69	0.07	0.07
query5	0.50	0.50	0.51
query6	1.13	0.73	0.71
query7	0.02	0.02	0.01
query8	0.05	0.05	0.05
query9	0.58	0.52	0.52
query10	0.56	0.58	0.58
query11	0.17	0.12	0.12
query12	0.15	0.13	0.12
query13	0.62	0.61	0.59
query14	1.47	1.46	1.53
query15	0.90	0.87	0.87
query16	0.36	0.37	0.36
query17	1.01	1.03	1.08
query18	0.21	0.20	0.21
query19	1.93	1.77	1.88
query20	0.01	0.01	0.01
query21	15.40	0.68	0.69
query22	4.03	7.71	1.64
query23	17.96	1.33	1.34
query24	2.25	0.23	0.22
query25	0.18	0.08	0.09
query26	0.28	0.18	0.17
query27	0.07	0.08	0.08
query28	13.18	1.11	1.09
query29	12.55	3.38	3.36
query30	0.25	0.06	0.05
query31	2.88	0.42	0.41
query32	3.24	0.48	0.49
query33	3.01	3.11	3.06
query34	15.45	4.33	4.32
query35	4.35	4.33	4.31
query36	0.68	0.49	0.49
query37	0.19	0.16	0.17
query38	0.17	0.15	0.15
query39	0.04	0.03	0.04
query40	0.19	0.14	0.12
query41	0.10	0.05	0.06
query42	0.06	0.06	0.05
query43	0.05	0.04	0.05
Total cold run time: 108.26 s
Total hot run time: 31.47 s

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.89% (9461/25647)
Line Coverage: 28.24% (77764/275349)
Region Coverage: 27.66% (40172/145241)
Branch Coverage: 24.27% (20411/84110)
Coverage Report: http://coverage.selectdb-in.cc/coverage/8adddc72d3fdbee0abb8f4d3f1d8b9b546b64c2d_8adddc72d3fdbee0abb8f4d3f1d8b9b546b64c2d/report/index.html

@zclllyybb
Copy link
Contributor

This pr is replaced by #40769

@zclllyybb
Copy link
Contributor

zclllyybb commented Sep 12, 2024 •
edited
Loading

Why I test the regression test case on 2.1.6 and 2.0.13, both got the same result as the output in this pr.

Is there any other steps to reproduce the problem?

to reproduce it, you need an environment has at least 3 BEs

@mrhhsg
Copy link
Member Author

mrhhsg commented Sep 13, 2024

#40769

@mrhhsg mrhhsg closed this Sep 13, 2024
@zclllyybb zclllyybb mentioned this pull request Sep 14, 2024
Merged
HappenLee pushed a commit that referenced this pull request Sep 20, 2024
@zclllyybb
[Fix-2.0](column) Fix wrong has_null flag after filter_by_selector (#…
f4674f6 
…40849)

in master and branch-2.1 it fixed by refactor
#40769. for 2.0 we pick
#40756 as a minimal modification
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cambyzju cambyzju cambyzju left review comments

@zclllyybb zclllyybb zclllyybb approved these changes

Assignees
No one assigned
Labels
reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mrhhsg @doris-robot @cambyzju @zclllyybb @yiguolei @wm1581066