Skip to content

Conversation

@eldenmoon
Copy link
Member

@eldenmoon eldenmoon commented May 13, 2024

Proposed changes

  1. datatype->to_string result in incorrect string value, use serdes instead
  2. cast json int to json tinyint should not result null

Will result behavior change

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

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

@eldenmoon
Copy link
Member Author

run buildall

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@eldenmoon
Copy link
Member Author

run buildall

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@eldenmoon
Copy link
Member Author

run buildall

@eldenmoon
Copy link
Member Author

run buildall

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.67% (8990/25206)
Line Coverage: 27.31% (74268/271915)
Region Coverage: 26.55% (38393/144597)
Branch Coverage: 23.37% (19585/83794)
Coverage Report: http://coverage.selectdb-in.cc/coverage/c86291fb8d0766e763637005e5238435a9f28c96_c86291fb8d0766e763637005e5238435a9f28c96/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17611	4999	4313	4313
q2	2016	189	185	185
q3	10497	1149	1185	1149
q4	10196	740	852	740
q5	7511	2765	2785	2765
q6	225	131	137	131
q7	1021	594	593	593
q8	9227	2173	2115	2115
q9	9398	6723	6691	6691
q10	9071	3956	4002	3956
q11	497	244	250	244
q12	541	238	241	238
q13	17453	3195	3099	3099
q14	246	210	210	210
q15	511	471	488	471
q16	521	422	405	405
q17	981	650	762	650
q18	8392	7890	7766	7766
q19	5732	1561	1576	1561
q20	654	317	328	317
q21	5297	4716	4087	4087
q22	355	278	291	278
Total cold run time: 117953 ms
Total hot run time: 41964 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4470	4415	4498	4415
q2	375	260	276	260
q3	3130	2983	2796	2796
q4	1838	1584	1684	1584
q5	5538	5516	5539	5516
q6	217	129	127	127
q7	2356	1975	2027	1975
q8	3232	3408	3469	3408
q9	8746	8720	8584	8584
q10	3959	3848	3847	3847
q11	603	490	495	490
q12	840	623	604	604
q13	17098	3147	3146	3146
q14	277	247	237	237
q15	522	464	467	464
q16	454	402	431	402
q17	1771	1486	1459	1459
q18	7577	7643	7323	7323
q19	1660	1508	1570	1508
q20	1965	1758	1761	1758
q21	8976	4804	4800	4800
q22	589	488	481	481
Total cold run time: 76193 ms
Total hot run time: 55184 ms

@doris-robot
Copy link

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

query1	910	378	345	345
query2	6436	2351	2173	2173
query3	6657	206	210	206
query4	25008	21248	21128	21128
query5	4128	419	407	407
query6	262	172	166	166
query7	4586	289	287	287
query8	248	204	191	191
query9	8470	2425	2386	2386
query10	442	247	262	247
query11	14666	14075	14224	14075
query12	134	91	86	86
query13	1651	374	377	374
query14	10378	8314	7540	7540
query15	208	178	170	170
query16	7780	272	264	264
query17	1708	587	546	546
query18	1965	269	260	260
query19	209	165	146	146
query20	91	83	83	83
query21	193	128	127	127
query22	5070	4908	4912	4908
query23	34136	33512	33683	33512
query24	6668	2863	2891	2863
query25	521	375	361	361
query26	703	154	150	150
query27	1932	317	316	316
query28	3649	2064	2065	2064
query29	838	608	591	591
query30	229	154	156	154
query31	960	752	748	748
query32	90	55	54	54
query33	491	244	248	244
query34	859	471	487	471
query35	768	661	674	661
query36	1022	926	877	877
query37	101	65	64	64
query38	2874	2763	2757	2757
query39	1622	1544	1568	1544
query40	201	124	122	122
query41	42	38	39	38
query42	99	99	119	99
query43	572	529	554	529
query44	1062	716	737	716
query45	260	249	222	222
query46	1057	691	707	691
query47	1973	1868	1892	1868
query48	365	296	297	296
query49	763	390	401	390
query50	769	387	386	386
query51	6871	6745	6763	6745
query52	106	92	84	84
query53	348	281	279	279
query54	528	436	436	436
query55	74	73	71	71
query56	233	221	224	221
query57	1194	1152	1125	1125
query58	214	198	193	193
query59	3327	3334	3348	3334
query60	254	226	229	226
query61	89	88	86	86
query62	585	467	454	454
query63	304	278	285	278
query64	8216	7366	7348	7348
query65	3118	3059	3063	3059
query66	787	342	332	332
query67	15323	14950	14915	14915
query68	4562	534	524	524
query69	475	292	304	292
query70	1147	1147	1164	1147
query71	383	264	263	263
query72	7117	2521	2331	2331
query73	692	320	315	315
query74	6540	6121	6050	6050
query75	3296	2606	2608	2606
query76	2272	983	998	983
query77	372	268	258	258
query78	10529	10224	10329	10224
query79	1387	515	525	515
query80	906	483	459	459
query81	566	219	218	218
query82	997	94	90	90
query83	253	162	165	162
query84	250	83	84	83
query85	962	267	256	256
query86	409	317	312	312
query87	3275	3072	3088	3072
query88	3049	2321	2329	2321
query89	469	388	392	388
query90	1955	190	189	189
query91	129	97	96	96
query92	59	48	51	48
query93	1011	516	502	502
query94	1052	186	181	181
query95	394	301	312	301
query96	571	264	265	264
query97	3138	2967	2956	2956
query98	256	221	216	216
query99	1208	888	894	888
Total cold run time: 267078 ms
Total hot run time: 186599 ms

type_index == TypeIndex::Int64 ||
type_index == TypeIndex::Int128) {
if (value->isInt()) {
res[i] = ((const JsonbIntVal*)value)->val();
Copy link
Contributor

Choose a reason for hiding this comment

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

Will it cause wrong result, eg int64 -> int8 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

the same as cast bigint to tinyint

Copy link
Contributor

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

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label May 16, 2024
@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

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

query1	913	382	383	382
query2	6274	2495	2301	2301
query3	6664	217	211	211
query4	24147	21822	22041	21822
query5	3720	431	443	431
query6	273	191	175	175
query7	4544	299	302	299
query8	234	200	189	189
query9	8505	2414	2460	2414
query10	441	262	244	244
query11	15123	14871	14943	14871
query12	121	91	89	89
query13	1620	386	382	382
query14	10916	8407	7691	7691
query15	254	169	168	168
query16	8127	265	268	265
query17	1695	580	551	551
query18	2053	290	277	277
query19	263	159	154	154
query20	90	83	84	83
query21	193	127	130	127
query22	5027	4874	4860	4860
query23	34217	33811	33468	33468
query24	10763	2909	2958	2909
query25	628	393	389	389
query26	1274	158	154	154
query27	2870	332	314	314
query28	7509	2055	2046	2046
query29	882	625	629	625
query30	273	172	170	170
query31	961	753	741	741
query32	93	54	52	52
query33	741	262	255	255
query34	1016	486	477	477
query35	812	667	670	667
query36	1071	945	943	943
query37	139	73	76	73
query38	2870	2749	2782	2749
query39	1627	1692	1577	1577
query40	203	125	125	125
query41	49	45	43	43
query42	105	95	101	95
query43	603	537	551	537
query44	1182	751	761	751
query45	273	256	263	256
query46	1088	731	705	705
query47	1972	1872	1870	1870
query48	379	302	304	302
query49	917	396	405	396
query50	765	383	386	383
query51	6995	6785	6709	6709
query52	103	91	90	90
query53	353	292	289	289
query54	893	440	444	440
query55	79	80	76	76
query56	253	245	234	234
query57	1231	1151	1159	1151
query58	230	202	207	202
query59	3437	3459	3178	3178
query60	261	244	253	244
query61	110	108	107	107
query62	762	466	473	466
query63	315	293	284	284
query64	9749	7419	7335	7335
query65	3219	3083	3113	3083
query66	1387	337	335	335
query67	15616	15048	15311	15048
query68	9891	556	568	556
query69	607	309	315	309
query70	1395	1070	1155	1070
query71	545	273	275	273
query72	8701	2567	2397	2397
query73	1606	322	319	319
query74	6629	6181	6251	6181
query75	5565	2617	2600	2600
query76	5377	1020	1025	1020
query77	697	265	268	265
query78	10759	10292	10006	10006
query79	7055	520	512	512
query80	1051	428	425	425
query81	506	249	245	245
query82	236	93	98	93
query83	194	166	165	165
query84	264	90	82	82
query85	925	275	270	270
query86	342	321	310	310
query87	3316	3078	3124	3078
query88	4872	2425	2438	2425
query89	514	368	378	368
query90	2000	180	180	180
query91	125	96	97	96
query92	62	46	45	45
query93	5661	512	503	503
query94	1178	178	179	178
query95	386	304	301	301
query96	607	269	262	262
query97	3139	3010	3042	3010
query98	229	214	213	213
query99	1197	891	885	885
Total cold run time: 307103 ms
Total hot run time: 188971 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.03
query2	0.09	0.04	0.04
query3	0.23	0.05	0.04
query4	1.68	0.07	0.07
query5	0.50	0.48	0.50
query6	1.14	0.73	0.72
query7	0.02	0.02	0.01
query8	0.05	0.04	0.04
query9	0.54	0.49	0.49
query10	0.53	0.54	0.55
query11	0.15	0.11	0.11
query12	0.15	0.12	0.12
query13	0.60	0.59	0.59
query14	0.80	0.75	0.78
query15	0.82	0.81	0.82
query16	0.37	0.37	0.38
query17	1.03	1.01	0.97
query18	0.21	0.25	0.23
query19	1.77	1.81	1.66
query20	0.01	0.01	0.01
query21	15.51	0.68	0.67
query22	4.92	6.10	2.23
query23	18.29	1.35	1.24
query24	1.60	0.30	0.23
query25	0.15	0.07	0.08
query26	0.25	0.16	0.16
query27	0.08	0.08	0.07
query28	13.29	1.02	1.00
query29	13.23	3.27	3.25
query30	0.24	0.06	0.05
query31	2.88	0.38	0.38
query32	3.29	0.48	0.48
query33	2.86	2.81	2.89
query34	17.06	4.41	4.35
query35	4.48	4.45	4.64
query36	0.66	0.47	0.46
query37	0.17	0.15	0.15
query38	0.15	0.15	0.14
query39	0.04	0.03	0.03
query40	0.17	0.14	0.14
query41	0.10	0.05	0.04
query42	0.05	0.04	0.05
query43	0.04	0.03	0.04
Total cold run time: 110.24 s
Total hot run time: 30.55 s

@eldenmoon
Copy link
Member Author

run buildall

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.70% (9006/25227)
Line Coverage: 27.35% (74455/272224)
Region Coverage: 26.61% (38524/144760)
Branch Coverage: 23.45% (19665/83868)
Coverage Report: http://coverage.selectdb-in.cc/coverage/f33dd1341ad1c572778a1954550eb43be2f06696_f33dd1341ad1c572778a1954550eb43be2f06696/report/index.html

Copy link
Contributor

@qidaye qidaye left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants