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](prepared statement) fix protocol with TIME datatype #47389

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

eldenmoon
Copy link
Member

@eldenmoon eldenmoon commented Jan 24, 2025

What problem does this PR solve?

This pull request adds functionality to encode time values into MySQL binary protocol format with microsecond precision and includes corresponding test cases. The changes primarily involve adding a new function for encoding time and updating the push_time and push_timev2 methods in the MysqlRowBuffer class to use this new function.

Encoding time into MySQL binary protocol format:

  • be/src/util/mysql_row_buffer.cpp: Added the encode_binary_timev2 function to encode time into MySQL binary protocol format with support for microsecond precision. The function handles time values within the range of '-838:59:59' to '838:59:59' and adjusts the precision based on the provided scale.

Updates to MysqlRowBuffer class:

Test cases:

  • be/test/util/mysql_row_buffer_test.cpp: Added a new test case TestBinaryTimeCompressedEncoding to verify the correct encoding of time values in various scenarios, including zero time, time without microseconds, time with microseconds, and negative time values.
    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 Jan 24, 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?

@eldenmoon
Copy link
Member Author

run buildall

@eldenmoon
Copy link
Member Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17572	5556	5421	5421
q2	2038	313	172	172
q3	10407	1258	736	736
q4	10217	964	535	535
q5	7513	2383	2196	2196
q6	202	172	135	135
q7	900	772	618	618
q8	9249	1362	1160	1160
q9	5174	4910	4889	4889
q10	6779	2322	1894	1894
q11	484	292	261	261
q12	352	357	224	224
q13	17768	3702	3046	3046
q14	236	225	210	210
q15	503	459	466	459
q16	606	618	597	597
q17	571	856	331	331
q18	7024	6580	6369	6369
q19	1202	954	544	544
q20	325	313	198	198
q21	2858	2185	2006	2006
q22	369	322	310	310
Total cold run time: 102349 ms
Total hot run time: 32311 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5525	5455	5556	5455
q2	235	324	236	236
q3	2289	2614	2301	2301
q4	1426	1811	1404	1404
q5	4308	4720	4632	4632
q6	161	155	121	121
q7	2060	1995	1800	1800
q8	2656	2815	2668	2668
q9	7303	7131	7275	7131
q10	2959	3272	2766	2766
q11	579	517	515	515
q12	712	743	578	578
q13	3552	3963	3277	3277
q14	267	294	286	286
q15	523	457	474	457
q16	646	697	645	645
q17	1214	1726	1279	1279
q18	7656	7454	7346	7346
q19	795	857	1086	857
q20	1988	2018	1848	1848
q21	5676	5416	5086	5086
q22	594	594	553	553
Total cold run time: 53124 ms
Total hot run time: 51241 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 184725 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 4934b0067b3d181b73ca83811dfd88aec2a89dfe, data reload: false

query1	968	377	386	377
query2	6520	2166	2055	2055
query3	6789	213	216	213
query4	33266	23431	23167	23167
query5	4499	625	477	477
query6	303	207	203	203
query7	4623	502	304	304
query8	295	255	234	234
query9	9452	2578	2578	2578
query10	499	324	254	254
query11	18246	15418	14961	14961
query12	158	105	107	105
query13	1639	540	385	385
query14	11390	6222	7369	6222
query15	230	194	191	191
query16	7854	654	422	422
query17	1583	743	561	561
query18	2091	397	290	290
query19	231	181	146	146
query20	114	112	110	110
query21	208	123	107	107
query22	4131	4419	4175	4175
query23	33762	32945	32976	32945
query24	6366	2305	2300	2300
query25	530	477	410	410
query26	1216	276	158	158
query27	1992	483	353	353
query28	5146	2420	2407	2407
query29	739	568	445	445
query30	243	199	164	164
query31	948	882	805	805
query32	84	63	63	63
query33	524	370	318	318
query34	761	861	569	569
query35	792	819	730	730
query36	988	1052	914	914
query37	119	95	77	77
query38	4221	4190	4141	4141
query39	1465	1417	1468	1417
query40	213	115	104	104
query41	52	51	49	49
query42	124	105	104	104
query43	527	528	483	483
query44	1315	815	809	809
query45	175	176	166	166
query46	866	1031	638	638
query47	1823	1820	1788	1788
query48	385	392	316	316
query49	782	497	404	404
query50	622	665	405	405
query51	4164	4129	4158	4129
query52	113	113	93	93
query53	231	267	198	198
query54	491	497	418	418
query55	85	79	80	79
query56	258	259	242	242
query57	1176	1110	1094	1094
query58	258	234	231	231
query59	3086	3276	3133	3133
query60	283	288	254	254
query61	118	112	119	112
query62	796	704	666	666
query63	221	189	195	189
query64	4325	1013	704	704
query65	3292	3162	3135	3135
query66	1085	429	308	308
query67	16005	15658	15327	15327
query68	4279	829	543	543
query69	470	290	270	270
query70	1221	1143	1107	1107
query71	403	291	259	259
query72	5921	3780	3775	3775
query73	656	757	364	364
query74	10387	9146	8763	8763
query75	3166	3183	2685	2685
query76	3245	1200	795	795
query77	478	348	273	273
query78	10013	10728	9556	9556
query79	2956	763	594	594
query80	816	527	466	466
query81	560	283	231	231
query82	310	153	131	131
query83	185	175	148	148
query84	238	97	73	73
query85	755	357	301	301
query86	424	306	292	292
query87	4485	4559	4359	4359
query88	4437	2212	2190	2190
query89	391	326	295	295
query90	1941	194	192	192
query91	140	142	109	109
query92	72	60	54	54
query93	1075	846	532	532
query94	743	419	296	296
query95	362	273	259	259
query96	491	599	285	285
query97	2731	2890	2734	2734
query98	234	202	191	191
query99	1299	1355	1272	1272
Total cold run time: 282740 ms
Total hot run time: 184725 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.07	0.03	0.03
query3	0.24	0.07	0.06
query4	1.62	0.11	0.11
query5	0.41	0.43	0.40
query6	1.18	0.65	0.66
query7	0.02	0.02	0.02
query8	0.04	0.03	0.04
query9	0.60	0.54	0.52
query10	0.56	0.57	0.57
query11	0.14	0.11	0.11
query12	0.14	0.11	0.11
query13	0.60	0.60	0.60
query14	2.85	2.74	2.73
query15	0.90	0.81	0.82
query16	0.38	0.39	0.38
query17	1.03	1.00	1.01
query18	0.24	0.22	0.21
query19	1.96	1.83	1.99
query20	0.01	0.01	0.02
query21	15.36	0.97	0.58
query22	0.76	0.75	0.66
query23	15.30	1.44	0.61
query24	3.00	1.27	1.25
query25	0.24	0.15	0.07
query26	0.29	0.15	0.14
query27	0.06	0.05	0.04
query28	13.96	1.01	0.43
query29	12.58	3.92	3.31
query30	0.25	0.10	0.06
query31	2.83	0.60	0.39
query32	3.23	0.55	0.48
query33	3.00	3.08	2.99
query34	16.39	5.21	4.52
query35	4.66	4.55	4.56
query36	0.64	0.50	0.53
query37	0.09	0.06	0.05
query38	0.04	0.04	0.03
query39	0.04	0.02	0.02
query40	0.17	0.13	0.12
query41	0.07	0.02	0.02
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 106.06 s
Total hot run time: 31.09 s

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 41.99% (10951/26081)
Line Coverage: 32.26% (92522/286779)
Region Coverage: 31.39% (47403/151021)
Branch Coverage: 27.45% (24016/87492)
Coverage Report: http://coverage.selectdb-in.cc/coverage/4934b0067b3d181b73ca83811dfd88aec2a89dfe_4934b0067b3d181b73ca83811dfd88aec2a89dfe/report/index.html

be/src/util/mysql_row_buffer.cpp Show resolved Hide resolved
MysqlRowBuffer<true> buffer;

// case1 : all 0, should only send 1 byte
buffer.push_timev2(0.0, 6);
Copy link
Contributor

Choose a reason for hiding this comment

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

check the contents of a buffer

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@eldenmoon
Copy link
Member Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17601	5440	5327	5327
q2	2048	294	163	163
q3	10459	1225	750	750
q4	10255	979	506	506
q5	8265	2363	2141	2141
q6	193	162	131	131
q7	892	747	622	622
q8	9215	1346	1154	1154
q9	5247	4908	4882	4882
q10	6848	2330	1886	1886
q11	484	275	261	261
q12	345	366	225	225
q13	17767	3708	3067	3067
q14	252	233	209	209
q15	508	466	456	456
q16	629	609	576	576
q17	563	852	329	329
q18	6753	6394	6302	6302
q19	1549	975	555	555
q20	334	331	196	196
q21	2990	2343	2080	2080
q22	370	353	323	323
Total cold run time: 103567 ms
Total hot run time: 32141 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5702	5381	5428	5381
q2	254	333	243	243
q3	2335	2637	2282	2282
q4	1377	1852	1400	1400
q5	4293	4763	4687	4687
q6	170	156	124	124
q7	2026	1942	1810	1810
q8	2643	2768	2752	2752
q9	7327	7193	7302	7193
q10	3036	3286	2786	2786
q11	600	519	494	494
q12	699	730	613	613
q13	3460	3900	3251	3251
q14	294	298	271	271
q15	519	479	478	478
q16	639	694	657	657
q17	1260	1742	1259	1259
q18	7594	7450	7381	7381
q19	834	1074	1104	1074
q20	1983	2033	1954	1954
q21	5767	5209	4824	4824
q22	590	594	546	546
Total cold run time: 53402 ms
Total hot run time: 51460 ms

@doris-robot
Copy link

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

query1	1320	924	920	920
query2	6315	2064	1990	1990
query3	11115	4636	4503	4503
query4	32851	23624	23026	23026
query5	3716	616	458	458
query6	279	199	203	199
query7	3980	490	297	297
query8	297	244	236	236
query9	9336	2686	2679	2679
query10	453	332	262	262
query11	17740	15112	15124	15112
query12	155	105	102	102
query13	1544	517	397	397
query14	10163	7194	6488	6488
query15	274	197	181	181
query16	7334	643	473	473
query17	1584	736	580	580
query18	1946	390	315	315
query19	217	180	163	163
query20	119	123	119	119
query21	210	137	113	113
query22	4530	4637	4284	4284
query23	34467	33521	33194	33194
query24	6585	2336	2362	2336
query25	513	457	393	393
query26	819	286	157	157
query27	2187	482	338	338
query28	5288	2535	2504	2504
query29	604	601	461	461
query30	221	193	163	163
query31	944	879	820	820
query32	83	67	63	63
query33	508	369	314	314
query34	772	864	531	531
query35	799	857	781	781
query36	1019	1061	976	976
query37	125	105	83	83
query38	4335	4329	4255	4255
query39	1484	1451	1468	1451
query40	221	117	108	108
query41	65	115	48	48
query42	128	103	105	103
query43	500	522	497	497
query44	1368	825	815	815
query45	186	190	170	170
query46	884	1061	671	671
query47	1879	1925	1884	1884
query48	396	413	339	339
query49	719	516	383	383
query50	667	684	411	411
query51	4377	4343	4253	4253
query52	113	105	97	97
query53	240	259	192	192
query54	497	571	416	416
query55	80	81	83	81
query56	250	257	260	257
query57	1215	1192	1108	1108
query58	252	254	230	230
query59	3047	3217	2951	2951
query60	279	302	260	260
query61	114	116	115	115
query62	786	723	663	663
query63	234	194	192	192
query64	3430	1033	701	701
query65	3308	3270	3251	3251
query66	714	429	301	301
query67	16280	15658	15431	15431
query68	6765	830	524	524
query69	496	293	250	250
query70	1176	1144	1123	1123
query71	377	317	275	275
query72	5108	4040	3868	3868
query73	649	745	373	373
query74	9890	9040	8833	8833
query75	3209	3127	2617	2617
query76	3142	1166	772	772
query77	472	437	273	273
query78	10160	9908	9317	9317
query79	2475	813	599	599
query80	1376	521	445	445
query81	551	284	243	243
query82	363	152	122	122
query83	189	170	150	150
query84	233	93	79	79
query85	751	417	301	301
query86	429	327	303	303
query87	4651	4666	4371	4371
query88	4238	2192	2206	2192
query89	396	324	291	291
query90	1717	192	192	192
query91	136	140	109	109
query92	74	54	53	53
query93	2803	869	541	541
query94	723	414	293	293
query95	327	266	251	251
query96	503	613	277	277
query97	2818	2854	2790	2790
query98	224	203	204	203
query99	1306	1370	1298	1298
Total cold run time: 286187 ms
Total hot run time: 190973 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 42.11% (11016/26162)
Line Coverage: 32.38% (92994/287188)
Region Coverage: 31.54% (47698/151236)
Branch Coverage: 27.54% (24127/87592)
Coverage Report: http://coverage.selectdb-in.cc/coverage/cdd68be64d83038500f370b0536868d95ebd8cfa_cdd68be64d83038500f370b0536868d95ebd8cfa/report/index.html

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.07	0.03	0.03
query3	0.24	0.07	0.06
query4	1.62	0.10	0.10
query5	0.42	0.40	0.40
query6	1.17	0.65	0.65
query7	0.02	0.02	0.02
query8	0.05	0.04	0.03
query9	0.59	0.51	0.48
query10	0.56	0.55	0.56
query11	0.14	0.10	0.11
query12	0.14	0.11	0.11
query13	0.62	0.59	0.60
query14	2.84	2.88	2.76
query15	0.88	0.81	0.81
query16	0.38	0.38	0.39
query17	1.07	0.99	0.99
query18	0.23	0.20	0.22
query19	1.81	1.88	1.96
query20	0.01	0.02	0.02
query21	15.39	0.99	0.60
query22	0.75	0.75	0.56
query23	15.38	1.40	0.58
query24	2.70	1.53	1.47
query25	0.19	0.26	0.08
query26	0.27	0.15	0.14
query27	0.05	0.05	0.05
query28	14.32	1.01	0.44
query29	12.57	3.97	3.26
query30	0.25	0.10	0.06
query31	2.81	0.61	0.37
query32	3.23	0.55	0.46
query33	2.96	3.00	3.06
query34	16.79	5.20	4.54
query35	4.54	4.57	4.54
query36	0.66	0.49	0.48
query37	0.10	0.06	0.05
query38	0.05	0.04	0.04
query39	0.03	0.02	0.02
query40	0.16	0.14	0.12
query41	0.08	0.03	0.02
query42	0.03	0.03	0.02
query43	0.03	0.03	0.03
Total cold run time: 106.23 s
Total hot run time: 31.12 s

@eldenmoon eldenmoon requested a review from csun5285 February 6, 2025 01:33
@xiaokang xiaokang changed the title [fix](prepared statement) fix protocol with time type [fix](prepared statement) fix protocol with TIME datatype Feb 6, 2025
@eldenmoon eldenmoon requested a review from HappenLee February 6, 2025 05:59
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

Copy link
Contributor

github-actions bot commented Feb 6, 2025

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

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Feb 6, 2025
Copy link
Contributor

github-actions bot commented Feb 6, 2025

PR approved by anyone and no changes requested.

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

@eldenmoon eldenmoon merged commit e00ae81 into apache:master Feb 6, 2025
26 of 29 checks passed
@eldenmoon eldenmoon deleted the fix-prep-timetype branch February 6, 2025 06:31
github-actions bot pushed a commit that referenced this pull request Feb 6, 2025
This pull request adds functionality to encode time values into MySQL
binary protocol format with microsecond precision and includes
corresponding test cases. The changes primarily involve adding a new
function for encoding time and updating the `push_time` and
`push_timev2` methods in the `MysqlRowBuffer` class to use this new
function.

### Encoding time into MySQL binary protocol format:

*
[`be/src/util/mysql_row_buffer.cpp`](diffhunk://#diff-181b7a490933fd9d3f8a00c97852d8fa17ab34694b1683ffd762e0f595a7c4fbR364-R433):
Added the `encode_binary_timev2` function to encode time into MySQL
binary protocol format with support for microsecond precision. The
function handles time values within the range of '-838:59:59' to
'838:59:59' and adjusts the precision based on the provided scale.

### Updates to `MysqlRowBuffer` class:

*
[`be/src/util/mysql_row_buffer.cpp`](diffhunk://#diff-181b7a490933fd9d3f8a00c97852d8fa17ab34694b1683ffd762e0f595a7c4fbR364-R433):
Updated the `push_time` method to throw an exception for unsupported
time types in binary protocol.
*
[`be/src/util/mysql_row_buffer.cpp`](diffhunk://#diff-181b7a490933fd9d3f8a00c97852d8fa17ab34694b1683ffd762e0f595a7c4fbL382-R451):
Modified the `push_timev2` method to use the new `encode_binary_timev2`
function, adjusting the buffer size and appending the encoded time.

### Test cases:

*
[`be/test/util/mysql_row_buffer_test.cpp`](diffhunk://#diff-1feab58caea3546e0e6fda52bff2fbd5f60f7cafcef8f891be83b0a2f74a93ebR118-R133):
Added a new test case `TestBinaryTimeCompressedEncoding` to verify the
correct encoding of time values in various scenarios, including zero
time, time without microseconds, time with microseconds, and negative
time values.
github-actions bot pushed a commit that referenced this pull request Feb 6, 2025
This pull request adds functionality to encode time values into MySQL
binary protocol format with microsecond precision and includes
corresponding test cases. The changes primarily involve adding a new
function for encoding time and updating the `push_time` and
`push_timev2` methods in the `MysqlRowBuffer` class to use this new
function.

### Encoding time into MySQL binary protocol format:

*
[`be/src/util/mysql_row_buffer.cpp`](diffhunk://#diff-181b7a490933fd9d3f8a00c97852d8fa17ab34694b1683ffd762e0f595a7c4fbR364-R433):
Added the `encode_binary_timev2` function to encode time into MySQL
binary protocol format with support for microsecond precision. The
function handles time values within the range of '-838:59:59' to
'838:59:59' and adjusts the precision based on the provided scale.

### Updates to `MysqlRowBuffer` class:

*
[`be/src/util/mysql_row_buffer.cpp`](diffhunk://#diff-181b7a490933fd9d3f8a00c97852d8fa17ab34694b1683ffd762e0f595a7c4fbR364-R433):
Updated the `push_time` method to throw an exception for unsupported
time types in binary protocol.
*
[`be/src/util/mysql_row_buffer.cpp`](diffhunk://#diff-181b7a490933fd9d3f8a00c97852d8fa17ab34694b1683ffd762e0f595a7c4fbL382-R451):
Modified the `push_timev2` method to use the new `encode_binary_timev2`
function, adjusting the buffer size and appending the encoded time.

### Test cases:

*
[`be/test/util/mysql_row_buffer_test.cpp`](diffhunk://#diff-1feab58caea3546e0e6fda52bff2fbd5f60f7cafcef8f891be83b0a2f74a93ebR118-R133):
Added a new test case `TestBinaryTimeCompressedEncoding` to verify the
correct encoding of time values in various scenarios, including zero
time, time without microseconds, time with microseconds, and negative
time values.
yiguolei pushed a commit that referenced this pull request Feb 8, 2025
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 reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants