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

branch-3.0: [fix](func) Fix precision loss in ST_GeometryFromWKB coordinate parsing #46661 #47264

Open
wants to merge 4 commits into
base: branch-3.0
Choose a base branch
from
Open

branch-3.0: [fix](func) Fix precision loss in ST_GeometryFromWKB coordinate parsing #46661 #47264

wants to merge 4 commits into from
+168 −76

Conversation

github-actions[bot]
Copy link
Contributor

Cherry-picked from #46661

@felixwluo
[fix](func) Fix precision loss in ST_GeometryFromWKB coordinate parsi…
8438c51 
…ng (#46661)

### What problem does this PR solve?

Issue Number: close #46619

Root Cause:
```
1. Unnecessary floating-point number conversions in coordinate handling:
   1.1 converting double to string using absl::StrFormat
   1.2 converting string back to double using std::stod
each conversion caused precision loss.

2. Byte order handling issue in WKB parsing:
    2.1 using machine endian before properly reading WKB byte order flag
    2.2 this caused incorrect interpretation of coordinate values
```

Solution:
```
1. Remove unnecessary coordinate value conversions:
    1.1 directly use S2LatLng's degrees() value without string formatting
    1.2 increase output precision in print_s2point to 15 digits

2. Fix WKB byte order handling:
    2.1 read byte order flag first
    2.2 set correct byte order before parsing coordinates
```

Result:
 before:
 ```
 POINT (1.461652102e-231 3.34424828009e-59)
 ```
 after:
 ```
 POINT(117.194767000297 36.46326301008)
 ```
@github-actions github-actions bot requested a review from dataroaring as a code owner January 21, 2025 06:46
@Thearas
Copy link
Contributor

Thearas commented Jan 21, 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?

@dataroaring dataroaring reopened this Jan 21, 2025
@Thearas
Copy link
Contributor

Thearas commented Jan 21, 2025

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17604	8014	7388	7388
q2	2492	171	163	163
q3	11302	1150	1238	1150
q4	10811	786	708	708
q5	7893	2986	2952	2952
q6	245	155	153	153
q7	1014	668	624	624
q8	9618	1986	2068	1986
q9	6940	6516	6469	6469
q10	7050	2299	2287	2287
q11	476	274	274	274
q12	411	220	208	208
q13	17812	3031	2990	2990
q14	247	205	208	205
q15	574	529	524	524
q16	689	619	616	616
q17	983	620	563	563
q18	7288	6708	6632	6632
q19	1379	1173	1002	1002
q20	483	202	195	195
q21	4115	3228	3283	3228
q22	1073	989	1002	989
Total cold run time: 110499 ms
Total hot run time: 41306 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7331	7344	7365	7344
q2	360	248	249	248
q3	3279	3014	3001	3001
q4	2148	1987	1836	1836
q5	5775	5717	5702	5702
q6	231	145	147	145
q7	2242	1803	1814	1803
q8	3342	3601	3527	3527
q9	8974	8851	8899	8851
q10	3606	3528	3548	3528
q11	588	503	515	503
q12	811	591	612	591
q13	3957	3154	3156	3154
q14	301	293	279	279
q15	580	520	525	520
q16	726	663	663	663
q17	1836	1637	1596	1596
q18	8166	7833	7643	7643
q19	1649	1525	1545	1525
q20	2076	1805	1799	1799
q21	5355	5174	5138	5138
q22	1056	1023	980	980
Total cold run time: 64389 ms
Total hot run time: 60376 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 192494 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 8438c51c5b2451d7c385e9858c2bcb3258d54ffc, data reload: false

query1	962	376	366	366
query2	6513	2119	2109	2109
query3	6709	220	211	211
query4	34108	23582	23506	23506
query5	4432	467	461	461
query6	283	191	184	184
query7	4620	314	307	307
query8	293	234	216	216
query9	9717	2673	2669	2669
query10	494	266	265	265
query11	18317	15448	15331	15331
query12	149	105	105	105
query13	1636	423	415	415
query14	9184	7397	7677	7397
query15	315	178	185	178
query16	8216	448	479	448
query17	1741	589	564	564
query18	2150	312	330	312
query19	387	161	157	157
query20	119	110	111	110
query21	212	108	108	108
query22	4337	4256	4121	4121
query23	34870	33986	34270	33986
query24	11847	2906	2938	2906
query25	720	424	413	413
query26	1855	180	178	178
query27	2839	344	337	337
query28	7731	2412	2411	2411
query29	1109	468	453	453
query30	338	172	165	165
query31	1017	793	819	793
query32	105	61	63	61
query33	807	314	320	314
query34	969	523	531	523
query35	945	732	712	712
query36	1128	946	923	923
query37	292	78	81	78
query38	4055	3931	3893	3893
query39	1483	1426	1451	1426
query40	286	105	107	105
query41	54	51	51	51
query42	112	102	103	102
query43	550	487	491	487
query44	1236	806	805	805
query45	187	173	171	171
query46	1191	748	710	710
query47	1897	1821	1812	1812
query48	469	381	382	381
query49	1314	425	407	407
query50	880	434	415	415
query51	7273	7059	7173	7059
query52	107	92	92	92
query53	279	192	186	186
query54	1175	472	479	472
query55	81	78	80	78
query56	312	250	247	247
query57	1265	1120	1093	1093
query58	236	225	236	225
query59	3174	3079	2897	2897
query60	291	267	256	256
query61	118	112	116	112
query62	858	674	671	671
query63	230	196	186	186
query64	5221	695	654	654
query65	3299	3371	3217	3217
query66	1445	327	320	320
query67	16064	15681	15325	15325
query68	4871	582	582	582
query69	437	268	284	268
query70	1147	1094	1098	1094
query71	382	268	263	263
query72	6395	4154	4075	4075
query73	813	345	349	345
query74	10259	8949	8953	8949
query75	3454	2668	2689	2668
query76	2886	1182	1055	1055
query77	432	290	273	273
query78	10592	9601	9589	9589
query79	1166	602	609	602
query80	776	451	433	433
query81	517	240	256	240
query82	1276	123	117	117
query83	245	146	153	146
query84	232	87	85	85
query85	1125	309	291	291
query86	341	300	285	285
query87	4406	4354	4357	4354
query88	3542	2412	2366	2366
query89	405	302	292	292
query90	2121	192	190	190
query91	187	154	155	154
query92	63	57	54	54
query93	1081	553	543	543
query94	830	272	296	272
query95	363	260	261	260
query96	676	295	281	281
query97	3321	3206	3195	3195
query98	214	210	203	203
query99	1550	1328	1322	1322
Total cold run time: 302713 ms
Total hot run time: 192494 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.02
query2	0.06	0.03	0.03
query3	0.23	0.07	0.07
query4	1.63	0.10	0.10
query5	0.52	0.52	0.52
query6	1.13	0.72	0.72
query7	0.02	0.02	0.01
query8	0.04	0.03	0.04
query9	0.57	0.50	0.49
query10	0.54	0.57	0.55
query11	0.14	0.10	0.11
query12	0.14	0.12	0.11
query13	0.60	0.60	0.58
query14	2.94	2.95	2.94
query15	0.91	0.84	0.82
query16	0.40	0.37	0.39
query17	1.03	0.98	1.03
query18	0.24	0.21	0.22
query19	1.94	1.82	2.02
query20	0.01	0.01	0.02
query21	15.36	0.62	0.61
query22	2.60	2.63	2.72
query23	17.04	0.86	0.93
query24	3.16	0.38	1.37
query25	0.26	0.16	0.06
query26	0.40	0.14	0.13
query27	0.05	0.04	0.04
query28	10.80	1.11	1.07
query29	12.60	3.28	3.28
query30	0.25	0.07	0.06
query31	2.86	0.39	0.38
query32	3.25	0.46	0.46
query33	3.04	3.04	3.01
query34	17.19	4.44	4.52
query35	4.53	4.58	4.55
query36	0.67	0.48	0.48
query37	0.10	0.06	0.06
query38	0.04	0.04	0.04
query39	0.03	0.02	0.02
query40	0.16	0.12	0.11
query41	0.08	0.02	0.02
query42	0.04	0.02	0.02
query43	0.04	0.02	0.03
Total cold run time: 107.68 s
Total hot run time: 33.21 s

@felixwluo
Copy link
Contributor

run cloud_p0

@felixwluo
Copy link
Contributor

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17574	7317	7289	7289
q2	2066	173	166	166
q3	10563	1042	1163	1042
q4	10504	774	708	708
q5	7744	2818	2777	2777
q6	233	144	146	144
q7	952	616	615	615
q8	9353	1940	1973	1940
q9	6581	6391	6402	6391
q10	7018	2319	2299	2299
q11	467	260	261	260
q12	410	212	209	209
q13	17776	2973	3000	2973
q14	239	206	213	206
q15	583	532	531	531
q16	673	600	594	594
q17	971	588	558	558
q18	7223	6797	6688	6688
q19	1400	1103	1059	1059
q20	487	213	200	200
q21	4002	3109	3316	3109
q22	1115	983	994	983
Total cold run time: 107934 ms
Total hot run time: 40741 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7303	7254	7197	7197
q2	333	240	239	239
q3	2977	2915	2933	2915
q4	1995	1878	1818	1818
q5	5667	5683	5687	5683
q6	228	140	142	140
q7	2253	1814	1809	1809
q8	3329	3545	3575	3545
q9	8836	8914	8786	8786
q10	3627	3591	3567	3567
q11	594	506	499	499
q12	797	612	587	587
q13	7771	3195	3160	3160
q14	304	291	273	273
q15	578	534	524	524
q16	684	649	669	649
q17	1818	1643	1625	1625
q18	8261	7791	7742	7742
q19	1663	1499	1502	1499
q20	2049	1834	1902	1834
q21	5457	5365	5372	5365
q22	1141	1072	1045	1045
Total cold run time: 67665 ms
Total hot run time: 60501 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 199089 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 430bf6d1d75f4820337dc30e69b5c97ff02b5279, data reload: false

query1	1276	894	886	886
query2	6247	2056	2112	2056
query3	11007	4336	4433	4336
query4	62807	29680	23751	23751
query5	5185	478	434	434
query6	379	168	172	168
query7	5477	307	312	307
query8	306	231	225	225
query9	8587	2653	2667	2653
query10	462	260	254	254
query11	17706	15327	15764	15327
query12	153	103	105	103
query13	1419	439	433	433
query14	10448	7707	7701	7701
query15	208	186	180	180
query16	7286	520	526	520
query17	1121	606	587	587
query18	1910	314	331	314
query19	202	159	157	157
query20	114	114	105	105
query21	206	120	107	107
query22	4642	4655	4668	4655
query23	34545	34306	34056	34056
query24	6345	2899	2864	2864
query25	524	407	426	407
query26	649	165	168	165
query27	1755	356	365	356
query28	4189	2450	2400	2400
query29	734	442	431	431
query30	242	178	160	160
query31	999	813	835	813
query32	68	55	58	55
query33	409	281	282	281
query34	915	503	527	503
query35	840	736	739	736
query36	1086	946	988	946
query37	116	68	71	68
query38	4088	3994	3987	3987
query39	1499	1499	1463	1463
query40	197	101	97	97
query41	51	46	50	46
query42	115	98	102	98
query43	540	499	486	486
query44	1181	810	796	796
query45	194	169	170	169
query46	1139	722	721	721
query47	2032	1969	1963	1963
query48	465	380	396	380
query49	709	390	396	390
query50	828	432	440	432
query51	7296	7252	7208	7208
query52	98	92	89	89
query53	256	179	179	179
query54	571	462	461	461
query55	81	79	76	76
query56	273	247	244	244
query57	1303	1182	1179	1179
query58	229	210	228	210
query59	3307	3084	3001	3001
query60	262	239	245	239
query61	110	104	104	104
query62	827	705	717	705
query63	216	184	182	182
query64	1365	669	634	634
query65	3251	3167	3205	3167
query66	692	303	303	303
query67	16052	15476	15541	15476
query68	3740	569	565	565
query69	418	263	262	262
query70	1192	1156	1123	1123
query71	353	262	270	262
query72	6390	4288	4147	4147
query73	750	346	359	346
query74	10389	9248	9489	9248
query75	3370	2621	2686	2621
query76	1911	1089	1033	1033
query77	489	276	273	273
query78	10538	9626	9540	9540
query79	2122	581	596	581
query80	1411	431	424	424
query81	533	242	239	239
query82	1213	119	115	115
query83	264	145	143	143
query84	278	77	72	72
query85	1050	292	293	292
query86	395	296	301	296
query87	4440	4299	4330	4299
query88	3730	2414	2382	2382
query89	410	294	292	292
query90	1965	183	185	183
query91	182	144	146	144
query92	63	49	50	49
query93	2766	545	549	545
query94	778	309	306	306
query95	352	263	252	252
query96	607	278	277	277
query97	3350	3185	3257	3185
query98	229	202	198	198
query99	1708	1410	1415	1410
Total cold run time: 317177 ms
Total hot run time: 199089 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.06	0.03	0.03
query3	0.24	0.06	0.06
query4	1.62	0.10	0.11
query5	0.51	0.52	0.53
query6	1.13	0.73	0.74
query7	0.02	0.02	0.02
query8	0.05	0.02	0.03
query9	0.57	0.50	0.50
query10	0.55	0.55	0.56
query11	0.14	0.10	0.10
query12	0.14	0.11	0.11
query13	0.60	0.59	0.60
query14	2.78	2.74	2.87
query15	0.89	0.83	0.83
query16	0.40	0.39	0.38
query17	1.05	1.07	1.01
query18	0.23	0.23	0.22
query19	1.82	1.82	2.06
query20	0.01	0.02	0.01
query21	15.37	0.57	0.60
query22	2.65	1.81	1.84
query23	17.05	0.94	0.79
query24	3.02	1.57	1.15
query25	0.32	0.19	0.09
query26	0.38	0.14	0.13
query27	0.04	0.04	0.04
query28	10.03	1.10	1.07
query29	12.62	3.21	3.23
query30	0.25	0.06	0.06
query31	2.86	0.39	0.40
query32	3.23	0.45	0.46
query33	2.98	3.02	3.01
query34	16.81	4.53	4.50
query35	4.52	4.57	4.52
query36	0.68	0.50	0.49
query37	0.10	0.06	0.06
query38	0.04	0.04	0.04
query39	0.04	0.02	0.02
query40	0.17	0.13	0.12
query41	0.08	0.02	0.02
query42	0.04	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 106.15 s
Total hot run time: 32.96 s

@felixwluo
Copy link
Contributor

run p0

@felixwluo
Copy link
Contributor

run buildall

@felixwluo
Copy link
Contributor

run buildall

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

@dataroaring dataroaring Awaiting requested review from dataroaring dataroaring is a code owner

At least 1 approving review is required to merge this pull request.

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

Successfully merging this pull request may close these issues.
4 participants
@Thearas @doris-robot @felixwluo @dataroaring