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](function) Fix wrong nullable processing for convert_tz #38764

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

zclllyybb
Copy link
Contributor

@zclllyybb zclllyybb commented Aug 2, 2024

Proposed changes

Issue Number: close #xxx

This pr make convert_tz use default nullity processing logic rather than update_null_map manually to get correct result. The bug of update_null_map will be fixed by #38787

before:

mysql> select convert_tz(dt, '+00:00', IF(property_value IS NULL, '+00:00', property_value)) from cvt_tz;
+--------------------------------------------------------------------------------+
| convert_tz(dt, '+00:00', if(property_value IS NULL, '+00:00', property_value)) |
+--------------------------------------------------------------------------------+
| 2024-04-18 23:20:00                                                            |
|                                                                                |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
+--------------------------------------------------------------------------------+

after:

mysql> select convert_tz(dt, '+00:00', IF(property_value IS NULL, '+00:00', property_value)) from cvt_tz;
+--------------------------------------------------------------------------------+
| convert_tz(dt, '+00:00', if(property_value IS NULL, '+00:00', property_value)) |
+--------------------------------------------------------------------------------+
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
+--------------------------------------------------------------------------------+

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

run buildall

@github-actions github-actions bot added the doing label Aug 2, 2024
Copy link
Contributor

github-actions bot commented Aug 2, 2024

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

Copy link
Contributor

github-actions bot commented Aug 2, 2024

PR approved by anyone and no changes requested.

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18541	4213	4168	4168
q2	2466	211	203	203
q3	11889	1382	1405	1382
q4	10630	894	997	894
q5	8023	3011	3013	3011
q6	221	140	140	140
q7	1064	616	608	608
q8	9437	1873	1992	1873
q9	8578	6617	6620	6617
q10	8761	3849	3863	3849
q11	438	251	253	251
q12	411	226	225	225
q13	17769	2962	2955	2955
q14	276	245	244	244
q15	526	486	493	486
q16	503	414	387	387
q17	975	947	948	947
q18	8070	7332	7365	7332
q19	1411	1223	1215	1215
q20	595	322	325	322
q21	5387	4781	4864	4781
q22	354	282	284	282
Total cold run time: 116325 ms
Total hot run time: 42172 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4045	4006	3993	3993
q2	329	225	225	225
q3	3005	3005	2991	2991
q4	1914	1913	1885	1885
q5	5273	5277	5293	5277
q6	215	130	134	130
q7	2070	1702	1703	1702
q8	3210	3284	3333	3284
q9	8381	8269	8347	8269
q10	3758	3855	3879	3855
q11	552	466	465	465
q12	722	540	556	540
q13	14176	2956	2966	2956
q14	289	264	259	259
q15	512	478	480	478
q16	447	421	421	421
q17	1753	1688	1682	1682
q18	7718	7351	7276	7276
q19	1676	1681	1692	1681
q20	1981	1786	1752	1752
q21	5480	5313	5219	5219
q22	543	472	459	459
Total cold run time: 68049 ms
Total hot run time: 54799 ms

@dataroaring dataroaring added usercase Important user case type label p0_w labels Aug 2, 2024
@doris-robot
Copy link

TPC-DS: Total hot run time: 168978 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 190570c37acc241813ec06d71f6567b8fd8cd973, data reload: false

query1	915	382	374	374
query2	6478	1618	1662	1618
query3	6681	214	216	214
query4	20185	17465	17391	17391
query5	4309	521	533	521
query6	308	188	168	168
query7	4594	322	303	303
query8	251	194	196	194
query9	8480	2349	2320	2320
query10	429	292	282	282
query11	10563	10109	10234	10109
query12	138	91	87	87
query13	1631	385	390	385
query14	8633	7234	6922	6922
query15	222	175	170	170
query16	7133	473	469	469
query17	957	569	562	562
query18	1928	289	288	288
query19	190	149	152	149
query20	96	85	90	85
query21	205	105	99	99
query22	4298	4207	3968	3968
query23	33564	32934	32940	32934
query24	10295	3068	2990	2990
query25	702	396	415	396
query26	1808	155	161	155
query27	2902	274	285	274
query28	6969	1965	1947	1947
query29	1360	432	431	431
query30	287	158	151	151
query31	939	761	757	757
query32	104	56	55	55
query33	709	321	324	321
query34	941	474	486	474
query35	869	725	721	721
query36	995	833	849	833
query37	290	80	81	80
query38	2889	2789	2766	2766
query39	873	814	819	814
query40	288	117	116	116
query41	49	47	47	47
query42	120	104	112	104
query43	454	412	421	412
query44	1186	737	755	737
query45	210	179	182	179
query46	1090	826	822	822
query47	1816	1708	1722	1708
query48	375	304	300	300
query49	1208	442	443	442
query50	915	445	448	445
query51	6703	6720	6658	6658
query52	99	92	87	87
query53	254	200	184	184
query54	650	473	462	462
query55	78	76	74	74
query56	289	266	273	266
query57	1177	1076	1063	1063
query58	286	279	277	277
query59	2544	2288	2242	2242
query60	324	276	299	276
query61	115	114	111	111
query62	924	665	669	665
query63	218	194	185	185
query64	5983	1947	1867	1867
query65	3209	3123	3101	3101
query66	1458	360	334	334
query67	15515	14803	14818	14803
query68	7093	576	589	576
query69	732	390	329	329
query70	1146	1084	1021	1021
query71	552	284	276	276
query72	8169	2685	2519	2519
query73	966	343	337	337
query74	6086	5663	5700	5663
query75	4685	2712	2738	2712
query76	5634	1275	1259	1259
query77	774	302	317	302
query78	9482	8865	8831	8831
query79	5386	537	541	537
query80	938	509	514	509
query81	552	218	229	218
query82	886	135	131	131
query83	328	172	174	172
query84	280	80	82	80
query85	1512	319	311	311
query86	420	269	306	269
query87	3299	3104	3141	3104
query88	4023	2513	2513	2513
query89	460	290	293	290
query90	2076	197	195	195
query91	129	100	105	100
query92	61	50	51	50
query93	4270	619	614	614
query94	958	288	288	288
query95	387	275	275	275
query96	633	286	285	285
query97	3298	3024	3108	3024
query98	218	199	192	192
query99	1614	1252	1344	1252
Total cold run time: 279704 ms
Total hot run time: 168978 ms

Copy link
Contributor

@zhiqiang-hhhh zhiqiang-hhhh left a comment

Choose a reason for hiding this comment

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

LGTM

@doris-robot
Copy link

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

query1	0.04	0.05	0.04
query2	0.07	0.04	0.03
query3	0.22	0.04	0.04
query4	1.68	0.08	0.07
query5	0.49	0.49	0.49
query6	1.14	0.72	0.71
query7	0.02	0.01	0.01
query8	0.05	0.04	0.05
query9	0.58	0.52	0.51
query10	0.56	0.56	0.58
query11	0.16	0.11	0.11
query12	0.15	0.12	0.13
query13	0.63	0.61	0.59
query14	0.77	0.79	0.80
query15	0.89	0.86	0.85
query16	0.36	0.36	0.34
query17	1.00	1.01	0.98
query18	0.21	0.21	0.21
query19	1.87	1.72	1.72
query20	0.01	0.00	0.01
query21	15.42	0.77	0.65
query22	3.78	7.89	1.48
query23	17.89	1.29	1.31
query24	2.23	0.23	0.22
query25	0.18	0.09	0.08
query26	0.31	0.22	0.21
query27	0.46	0.23	0.22
query28	13.19	1.02	0.97
query29	12.60	3.32	3.28
query30	0.28	0.06	0.06
query31	2.86	0.41	0.40
query32	3.23	0.49	0.48
query33	2.96	3.00	2.94
query34	15.40	4.25	4.30
query35	4.30	4.31	4.27
query36	0.68	0.48	0.49
query37	0.20	0.16	0.17
query38	0.16	0.15	0.16
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.05	0.05	0.04
Total cold run time: 107.44 s
Total hot run time: 30.08 s

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 Aug 5, 2024

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 Aug 5, 2024
@zhangstar333 zhangstar333 merged commit c60c8f5 into apache:master Aug 5, 2024
32 of 34 checks passed
dataroaring pushed a commit that referenced this pull request Aug 6, 2024
## Proposed changes

Issue Number: close #xxx

This pr make `convert_tz` use default nullity processing logic rather
than `update_null_map` manually to get correct result. The bug of
`update_null_map` will be fixed by
#38787

before:
```sql
mysql> select convert_tz(dt, '+00:00', IF(property_value IS NULL, '+00:00', property_value)) from cvt_tz;
+--------------------------------------------------------------------------------+
| convert_tz(dt, '+00:00', if(property_value IS NULL, '+00:00', property_value)) |
+--------------------------------------------------------------------------------+
| 2024-04-18 23:20:00                                                            |
|                                                                                |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
|                                                                                |
| 2024-04-18 23:20:00                                                            |
+--------------------------------------------------------------------------------+
```

after:
```sql
mysql> select convert_tz(dt, '+00:00', IF(property_value IS NULL, '+00:00', property_value)) from cvt_tz;
+--------------------------------------------------------------------------------+
| convert_tz(dt, '+00:00', if(property_value IS NULL, '+00:00', property_value)) |
+--------------------------------------------------------------------------------+
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
| 2024-04-18 23:20:00                                                            |
+--------------------------------------------------------------------------------+
```
@yiguolei yiguolei mentioned this pull request Sep 5, 2024
3 tasks
@zclllyybb zclllyybb deleted the fix_convert_tz branch September 6, 2024 04:14
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/3.0.1-merged doing reviewed usercase Important user case type label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants