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](Column) refactor ColumnNullable to provide flags safety #40769

Merged
merged 3 commits into from
Sep 14, 2024

Conversation

zclllyybb
Copy link
Contributor

@zclllyybb zclllyybb commented Sep 12, 2024

Proposed changes

Issue Number: close #xxx

In this pr we move null_map and relative flags to an individual base class to constrain its visibility.
please reed the comment carefully

test in master:

xxx/column_nullable_test.cpp:187: Failure
Value of: null_dst->has_null()
  Actual: false
Expected: true

Error occurred in case0
[  FAILED  ] ColumnNullableTest.PredicateTest (0 ms)
[ RUN      ] ColumnNullableTest.HashTest
[       OK ] ColumnNullableTest.HashTest (0 ms)
[----------] 3 tests from ColumnNullableTest (0 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (1 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ColumnNullableTest.PredicateTest

and success after this pr

Co-authored-by: Jerry Hu <[email protected]>
@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.

@zclllyybb zclllyybb changed the title [Refactor](Column) refactor ColumnNullable to provide flags safety [Fix](Column) refactor ColumnNullable to provide flags safety Sep 12, 2024
@zclllyybb
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.90% (9466/25651)
Line Coverage: 28.25% (77801/275368)
Region Coverage: 27.66% (40182/145248)
Branch Coverage: 24.28% (20421/84112)
Coverage Report: http://coverage.selectdb-in.cc/coverage/ee4233ecdb149c3da34b4772ed2370cede36e2e9_ee4233ecdb149c3da34b4772ed2370cede36e2e9/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17611	7329	7237	7237
q2	2033	186	176	176
q3	10603	1302	1420	1302
q4	10288	1043	1072	1043
q5	7719	3212	3190	3190
q6	239	152	148	148
q7	1060	634	631	631
q8	9477	2061	2063	2061
q9	6897	6307	6316	6307
q10	7032	2538	2517	2517
q11	440	260	255	255
q12	417	229	236	229
q13	17763	3052	3031	3031
q14	306	249	255	249
q15	589	542	523	523
q16	532	422	429	422
q17	998	954	977	954
q18	7576	6888	6841	6841
q19	1379	1252	1232	1232
q20	648	353	340	340
q21	3967	3580	3535	3535
q22	1099	1016	1008	1008
Total cold run time: 108673 ms
Total hot run time: 43231 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7167	7139	7191	7139
q2	350	247	249	247
q3	3134	3114	3061	3061
q4	2129	2153	2041	2041
q5	5705	5627	5781	5627
q6	248	148	150	148
q7	2207	1759	1768	1759
q8	3385	3448	3436	3436
q9	8849	8984	8817	8817
q10	3491	3586	3595	3586
q11	582	473	502	473
q12	830	614	628	614
q13	9203	3229	3176	3176
q14	302	281	278	278
q15	602	532	550	532
q16	530	457	468	457
q17	1822	1776	1748	1748
q18	8553	8107	8139	8107
q19	1776	1755	1766	1755
q20	2145	1927	1888	1888
q21	6036	5540	5681	5540
q22	1134	1054	1043	1043
Total cold run time: 70180 ms
Total hot run time: 61472 ms

@yiguolei yiguolei added usercase Important user case type label p0_b labels Sep 13, 2024
@zclllyybb
Copy link
Contributor Author

run buildall

Copy link
Contributor

@HappenLee HappenLee 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 Sep 13, 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.

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17591	7387	7238	7238
q2	2037	208	179	179
q3	10825	1278	1367	1278
q4	10303	1055	1069	1055
q5	7713	3207	3171	3171
q6	243	154	154	154
q7	1059	637	616	616
q8	9450	2080	2024	2024
q9	6782	6315	6309	6309
q10	7023	2547	2552	2547
q11	433	250	256	250
q12	429	231	232	231
q13	17761	3017	3029	3017
q14	285	241	256	241
q15	581	527	529	527
q16	518	424	451	424
q17	1021	967	972	967
q18	7525	6691	6856	6691
q19	1378	1242	1239	1239
q20	589	334	318	318
q21	3916	3619	3551	3551
q22	1049	988	1037	988
Total cold run time: 108511 ms
Total hot run time: 43015 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7211	7154	7194	7154
q2	346	233	247	233
q3	3118	3074	3109	3074
q4	2129	2103	2071	2071
q5	5772	5641	5753	5641
q6	239	149	152	149
q7	2148	1759	1771	1759
q8	3364	3477	3452	3452
q9	8865	8917	8876	8876
q10	3445	3607	3587	3587
q11	610	473	485	473
q12	794	595	640	595
q13	9470	3206	3206	3206
q14	309	294	277	277
q15	567	554	543	543
q16	516	461	462	461
q17	1790	1719	1714	1714
q18	8479	8061	8184	8061
q19	1787	1784	1754	1754
q20	2123	1880	1869	1869
q21	6044	5530	5725	5530
q22	1168	1062	1025	1025
Total cold run time: 70294 ms
Total hot run time: 61504 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.92% (9473/25658)
Line Coverage: 28.27% (77878/275445)
Region Coverage: 27.68% (40227/145307)
Branch Coverage: 24.29% (20441/84148)
Coverage Report: http://coverage.selectdb-in.cc/coverage/b5cbf4bcf9fe9117ecfc4c5c7dd763620d09180a_b5cbf4bcf9fe9117ecfc4c5c7dd763620d09180a/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18188	7945	7327	7327
q2	2628	193	200	193
q3	11454	1372	1313	1313
q4	11168	958	1057	958
q5	8090	3255	3203	3203
q6	245	163	157	157
q7	1046	632	610	610
q8	9447	2006	2016	2006
q9	6735	6268	6291	6268
q10	7038	2512	2501	2501
q11	428	254	254	254
q12	405	237	228	228
q13	17749	3057	3023	3023
q14	288	250	262	250
q15	575	522	511	511
q16	522	434	423	423
q17	1008	938	917	917
q18	7400	6804	6829	6804
q19	1385	1233	1226	1226
q20	609	349	338	338
q21	3887	3507	3492	3492
q22	1082	1005	1025	1005
Total cold run time: 111377 ms
Total hot run time: 43007 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7180	7154	7145	7145
q2	342	236	230	230
q3	2918	2906	2881	2881
q4	1974	1963	1967	1963
q5	5483	5436	5416	5416
q6	232	143	141	141
q7	2034	1684	1667	1667
q8	3223	3280	3346	3280
q9	8452	8429	8444	8429
q10	3385	3432	3456	3432
q11	570	454	469	454
q12	769	598	570	570
q13	5591	3048	3114	3048
q14	295	269	270	269
q15	573	514	521	514
q16	507	455	451	451
q17	1749	1716	1706	1706
q18	7976	7579	7666	7579
q19	1712	1693	1685	1685
q20	2057	1806	1817	1806
q21	5608	5527	5517	5517
q22	1108	1027	1021	1021
Total cold run time: 63738 ms
Total hot run time: 59204 ms

@doris-robot
Copy link

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

query1	946	389	388	388
query2	6460	1847	1790	1790
query3	6656	210	225	210
query4	26485	23887	23935	23887
query5	4799	550	519	519
query6	271	174	167	167
query7	4589	312	302	302
query8	276	220	230	220
query9	8531	2610	2614	2610
query10	468	286	295	286
query11	16322	15797	15527	15527
query12	159	102	102	102
query13	1692	410	387	387
query14	10929	7297	6747	6747
query15	215	178	173	173
query16	7572	461	468	461
query17	1560	578	569	569
query18	2000	306	299	299
query19	202	159	156	156
query20	125	113	114	113
query21	216	109	112	109
query22	4591	4242	4408	4242
query23	34603	33663	33812	33663
query24	9666	3185	3068	3068
query25	662	417	416	416
query26	818	161	160	160
query27	2178	281	286	281
query28	6241	2106	2093	2093
query29	935	425	428	425
query30	302	160	164	160
query31	1000	801	802	801
query32	108	56	64	56
query33	739	312	310	310
query34	923	496	493	493
query35	887	733	732	732
query36	1059	921	912	912
query37	146	87	87	87
query38	4030	3926	3973	3926
query39	1457	1411	1406	1406
query40	211	118	120	118
query41	51	50	47	47
query42	120	99	100	99
query43	489	442	461	442
query44	1234	799	782	782
query45	202	170	172	170
query46	1131	824	818	818
query47	1952	1796	1809	1796
query48	377	299	297	297
query49	1107	465	461	461
query50	931	434	453	434
query51	7036	7054	6805	6805
query52	101	89	89	89
query53	263	183	186	183
query54	775	487	483	483
query55	78	78	75	75
query56	294	288	285	285
query57	1236	1105	1093	1093
query58	249	257	249	249
query59	2844	2823	2753	2753
query60	329	281	280	280
query61	121	233	103	103
query62	905	689	679	679
query63	233	183	188	183
query64	5361	698	654	654
query65	3352	3161	3170	3161
query66	1408	305	304	304
query67	16107	15549	15550	15549
query68	3205	885	870	870
query69	433	329	332	329
query70	1174	1164	1138	1138
query71	359	352	347	347
query72	6068	3362	3348	3348
query73	634	589	592	589
query74	9310	9217	9178	9178
query75	3128	3018	3038	3018
query76	1945	874	858	858
query77	463	408	416	408
query78	9428	9297	9222	9222
query79	914	907	883	883
query80	898	873	827	827
query81	445	275	270	270
query82	267	268	271	268
query83	196	189	195	189
query84	227	112	108	108
query85	650	421	396	396
query86	312	311	317	311
query87	4360	4380	4427	4380
query88	4228	4181	4161	4161
query89	381	368	377	368
query90	1389	324	318	318
query91	134	125	123	123
query92	80	74	80	74
query93	1061	1081	1052	1052
query94	608	408	402	402
query95	466	437	421	421
query96	484	480	482	480
query97	3136	3144	3152	3144
query98	232	245	235	235
query99	1557	1306	1298	1298
Total cold run time: 277487 ms
Total hot run time: 195658 ms

@zclllyybb
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.91% (9471/25658)
Line Coverage: 28.27% (77859/275442)
Region Coverage: 27.68% (40216/145305)
Branch Coverage: 24.29% (20435/84146)
Coverage Report: http://coverage.selectdb-in.cc/coverage/b6baf07b14f278317e8c5437acf2f005f73349f7_b6baf07b14f278317e8c5437acf2f005f73349f7/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18068	7424	7341	7341
q2	2486	201	194	194
q3	11201	1506	1436	1436
q4	10792	1026	1026	1026
q5	7932	3312	3215	3215
q6	248	157	164	157
q7	1078	647	634	634
q8	9957	2048	2019	2019
q9	6834	6320	6356	6320
q10	7033	2520	2492	2492
q11	432	256	258	256
q12	409	233	226	226
q13	17775	3058	3014	3014
q14	283	254	257	254
q15	596	536	536	536
q16	530	427	429	427
q17	1017	971	970	970
q18	7566	7023	6690	6690
q19	1377	1262	1389	1262
q20	614	357	336	336
q21	3924	3546	3543	3543
q22	1088	973	1022	973
Total cold run time: 111240 ms
Total hot run time: 43321 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7201	7185	7192	7185
q2	344	234	225	225
q3	2966	2888	2914	2888
q4	1957	1981	1979	1979
q5	5461	5428	5516	5428
q6	238	148	148	148
q7	2058	1664	1680	1664
q8	3248	3349	3292	3292
q9	8462	8438	8448	8438
q10	3411	3486	3462	3462
q11	574	455	471	455
q12	756	574	598	574
q13	5753	3071	3052	3052
q14	294	272	273	272
q15	576	527	522	522
q16	487	450	458	450
q17	1779	1731	1740	1731
q18	8119	7463	7511	7463
q19	1741	1722	1705	1705
q20	2060	1791	1807	1791
q21	5675	5514	5508	5508
q22	1112	1016	1034	1016
Total cold run time: 64272 ms
Total hot run time: 59248 ms

@doris-robot
Copy link

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

query1	929	380	375	375
query2	6462	1768	1760	1760
query3	6687	210	227	210
query4	26005	23945	23857	23857
query5	5086	547	538	538
query6	261	170	170	170
query7	4596	313	322	313
query8	305	232	221	221
query9	8495	2613	2609	2609
query10	487	291	282	282
query11	16192	15430	15548	15430
query12	159	102	123	102
query13	1672	404	382	382
query14	11613	7051	7151	7051
query15	204	174	170	170
query16	7491	450	471	450
query17	1530	556	560	556
query18	1950	292	282	282
query19	186	145	144	144
query20	123	117	109	109
query21	214	104	101	101
query22	4475	4232	4380	4232
query23	34426	34168	33968	33968
query24	8779	3178	3084	3084
query25	612	396	413	396
query26	730	159	161	159
query27	2133	292	288	288
query28	5242	2104	2083	2083
query29	894	431	434	431
query30	298	159	154	154
query31	988	763	827	763
query32	108	58	62	58
query33	660	314	306	306
query34	908	478	489	478
query35	886	780	765	765
query36	1046	871	897	871
query37	149	82	88	82
query38	4062	3955	3929	3929
query39	1458	1424	1427	1424
query40	209	122	120	120
query41	53	51	49	49
query42	120	99	98	98
query43	485	444	447	444
query44	1205	802	778	778
query45	202	176	172	172
query46	1123	832	825	825
query47	1902	1780	1821	1780
query48	378	308	297	297
query49	1072	449	465	449
query50	909	448	449	448
query51	7002	6928	6898	6898
query52	106	90	88	88
query53	264	191	187	187
query54	748	480	483	480
query55	86	78	83	78
query56	292	288	267	267
query57	1234	1113	1094	1094
query58	260	239	255	239
query59	2828	2567	2769	2567
query60	307	296	283	283
query61	128	124	125	124
query62	917	668	704	668
query63	229	193	191	191
query64	5612	772	756	756
query65	3283	3170	3191	3170
query66	1446	320	313	313
query67	16208	15533	15488	15488
query68	3210	598	594	594
query69	425	280	281	280
query70	1145	1109	1081	1081
query71	342	299	286	286
query72	6146	4019	4060	4019
query73	763	325	338	325
query74	9280	9186	9046	9046
query75	3384	2699	2765	2699
query76	1926	1321	1308	1308
query77	441	318	325	318
query78	10019	9580	9303	9303
query79	1351	886	855	855
query80	1082	847	832	832
query81	519	270	266	266
query82	877	264	269	264
query83	218	194	190	190
query84	238	110	106	106
query85	737	420	402	402
query86	442	317	323	317
query87	4350	4361	4428	4361
query88	4209	4195	4124	4124
query89	383	370	376	370
query90	1726	326	318	318
query91	125	126	124	124
query92	79	78	75	75
query93	1111	1054	1052	1052
query94	817	387	406	387
query95	500	413	425	413
query96	484	479	477	477
query97	3171	3122	3147	3122
query98	233	230	224	224
query99	1594	1301	1303	1301
Total cold run time: 278436 ms
Total hot run time: 195773 ms

@doris-robot
Copy link

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

query1	0.04	0.05	0.04
query2	0.07	0.04	0.04
query3	0.22	0.05	0.05
query4	1.66	0.07	0.06
query5	0.51	0.49	0.50
query6	1.14	0.73	0.73
query7	0.02	0.02	0.02
query8	0.05	0.04	0.04
query9	0.57	0.52	0.51
query10	0.56	0.57	0.58
query11	0.16	0.12	0.12
query12	0.15	0.13	0.13
query13	0.63	0.62	0.61
query14	1.51	1.44	1.51
query15	0.90	0.88	0.89
query16	0.36	0.35	0.38
query17	1.06	1.04	1.00
query18	0.23	0.21	0.22
query19	1.89	1.81	1.83
query20	0.01	0.01	0.01
query21	15.39	0.69	0.68
query22	4.19	7.62	1.23
query23	17.91	1.33	1.35
query24	2.25	0.23	0.22
query25	0.18	0.08	0.08
query26	0.29	0.18	0.18
query27	0.08	0.09	0.08
query28	13.18	1.12	1.11
query29	12.52	3.35	3.30
query30	0.24	0.06	0.05
query31	2.87	0.42	0.42
query32	3.23	0.50	0.50
query33	3.05	3.03	3.09
query34	15.45	4.35	4.32
query35	4.37	4.36	4.37
query36	0.69	0.48	0.49
query37	0.18	0.15	0.17
query38	0.17	0.16	0.15
query39	0.05	0.04	0.04
query40	0.16	0.14	0.13
query41	0.10	0.05	0.05
query42	0.06	0.05	0.04
query43	0.05	0.04	0.05
Total cold run time: 108.4 s
Total hot run time: 31.08 s

@BiteTheDDDDt BiteTheDDDDt merged commit a953a51 into apache:master Sep 14, 2024
22 of 26 checks passed
@zclllyybb zclllyybb deleted the column_nullable branch September 14, 2024 02:22
zclllyybb added a commit to zclllyybb/doris that referenced this pull request Sep 14, 2024
…#40769)

Issue Number: close #xxx

In this pr we move null_map and relative flags to an individual base
class to constrain its visibility.
please reed the comment carefully

test in master:
```
xxx/column_nullable_test.cpp:187: Failure
Value of: null_dst->has_null()
  Actual: false
Expected: true

Error occurred in case0
[  FAILED  ] ColumnNullableTest.PredicateTest (0 ms)
[ RUN      ] ColumnNullableTest.HashTest
[       OK ] ColumnNullableTest.HashTest (0 ms)
[----------] 3 tests from ColumnNullableTest (0 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (1 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ColumnNullableTest.PredicateTest
```

and success after this pr

---------

Co-authored-by: Jerry Hu <[email protected]>
zclllyybb added a commit to zclllyybb/doris that referenced this pull request Sep 14, 2024
…#40769)

Issue Number: close #xxx

In this pr we move null_map and relative flags to an individual base
class to constrain its visibility.
please reed the comment carefully

test in master:
```
xxx/column_nullable_test.cpp:187: Failure
Value of: null_dst->has_null()
  Actual: false
Expected: true

Error occurred in case0
[  FAILED  ] ColumnNullableTest.PredicateTest (0 ms)
[ RUN      ] ColumnNullableTest.HashTest
[       OK ] ColumnNullableTest.HashTest (0 ms)
[----------] 3 tests from ColumnNullableTest (0 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (1 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ColumnNullableTest.PredicateTest
```

and success after this pr

---------

Co-authored-by: Jerry Hu <[email protected]>
HappenLee pushed a commit that referenced this pull request Sep 20, 2024
…40849)

in master and branch-2.1 it fixed by refactor
#40769. for 2.0 we pick
#40756 as a minimal modification
dataroaring pushed a commit that referenced this pull request Sep 26, 2024
## Proposed changes

Issue Number: close #xxx

In this pr we move null_map and relative flags to an individual base
class to constrain its visibility.
please reed the comment carefully

test in master:
```
xxx/column_nullable_test.cpp:187: Failure
Value of: null_dst->has_null()
  Actual: false
Expected: true

Error occurred in case0
[  FAILED  ] ColumnNullableTest.PredicateTest (0 ms)
[ RUN      ] ColumnNullableTest.HashTest
[       OK ] ColumnNullableTest.HashTest (0 ms)
[----------] 3 tests from ColumnNullableTest (0 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (1 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ColumnNullableTest.PredicateTest
```

and success after this pr

---------

Co-authored-by: Jerry Hu <[email protected]>
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.7-merged dev/3.0.2-merged p0_b reviewed usercase Important user case type label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants