[fix](s3) preserve explicit path style for object storage by sollhui · Pull Request #64034 · apache/doris

sollhui · 2026-06-02T14:50:12Z

What problem does this PR solve?

Related PR: #62023

OSS/COS/OBS filesystem providers translated their storage properties to S3-compatible properties, but always overwrote use_path_style with false. As a result, a user-provided use_path_style=true was lost before S3FileSystem and S3ObjStorage consumed the configuration. This made path-style behavior impossible to preserve through the provider path and could hide invalid path-style URL cases.

This change keeps the existing OSS/COS/OBS default of use_path_style=false when the option is not provided, but preserves an explicit user value by using putIfAbsent during provider creation and property translation. Unit tests cover both the translation helper and the final provider-created S3ObjStorage configuration.

sollhui · 2026-06-02T14:50:25Z

run buildall

sollhui · 2026-06-02T15:16:31Z

run buildall

hello-stephen · 2026-06-02T16:21:32Z

FE UT Coverage Report

Increment line coverage `` 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-06-02T16:38:18Z

TPC-H: Total hot run time: 29313 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit fdf08ce51ca12f7253a1314c0c0159fd5ad0d126, data reload: false
------ Round 1 ----------------------------------
orders	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL ============================================
q1	17684	4063	4052	4052
q2	q3	10760	1389	806	806
q4	4686	478	345	345
q5	7537	877	578	578
q6	186	175	138	138
q7	787	844	643	643
q8	9345	1607	1651	1607
q9	5930	4475	4495	4475
q10	6804	1853	1561	1561
q11	435	270	248	248
q12	623	429	284	284
q13	18129	3350	2799	2799
q14	261	258	238	238
q15	q16	799	776	711	711
q17	959	979	1010	979
q18	6955	5714	5517	5517
q19	1315	1282	994	994
q20	510	394	263	263
q21	6198	2768	2755	2755
q22	473	374	320	320
Total cold run time: 100376 ms
Total hot run time: 29313 ms
----- Round 2, with runtime_filter_mode=off -----
orders	Doris	NULL	NULL	150000000	42	6422171781	NULL	22778155	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL ============================================
q1	5155	4792	4786	4786
q2	q3	4861	5218	4654	4654
q4	2127	2178	1407	1407
q5	4668	4898	4703	4703
q6	229	183	128	128
q7	1902	1787	1630	1630
q8	2392	2073	2095	2073
q9	7882	7351	7335	7335
q10	4713	4705	4231	4231
q11	523	380	354	354
q12	721	742	524	524
q13	3016	3367	2788	2788
q14	282	274	256	256
q15	q16	675	696	604	604
q17	1279	1263	1242	1242
q18	7303	6785	6847	6785
q19	1157	1126	1122	1122
q20	2209	2232	1955	1955
q21	5342	4726	4475	4475
q22	518	448	441	441
Total cold run time: 56954 ms
Total hot run time: 51493 ms

hello-stephen · 2026-06-02T16:44:46Z

FE UT Coverage Report

Increment line coverage `` 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-06-02T16:49:09Z

TPC-DS: Total hot run time: 169466 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 fdf08ce51ca12f7253a1314c0c0159fd5ad0d126, data reload: false
query5	4337	643	482	482
query6	453	201	185	185
query7	4921	561	311	311
query8	372	218	204	204
query9	8813	3975	3973	3973
query10	473	301	266	266
query11	5921	2357	2134	2134
query12	157	104	96	96
query13	1246	605	464	464
query14	6400	5404	5088	5088
query14_1	4388	4397	4400	4397
query15	210	195	173	173
query16	983	430	413	413
query17	910	687	559	559
query18	2428	460	340	340
query19	192	179	138	138
query20	109	108	104	104
query21	222	132	113	113
query22	13626	13510	13422	13422
query23	17304	16520	16235	16235
query23_1	16205	16286	16209	16209
query24	7513	1757	1303	1303
query24_1	1296	1291	1301	1291
query25	547	443	374	374
query26	1294	314	168	168
query27	2727	570	323	323
query28	4483	2013	1968	1968
query29	1060	600	478	478
query30	303	268	195	195
query31	1116	1064	944	944
query32	103	59	60	59
query33	535	314	239	239
query34	1193	1136	657	657
query35	746	773	677	677
query36	1367	1415	1256	1256
query37	153	102	91	91
query38	3231	3165	3054	3054
query39	928	916	928	916
query39_1	882	910	883	883
query40	222	116	98	98
query41	76	64	61	61
query42	94	95	94	94
query43	318	328	279	279
query44 query45	193	190	184	184
query46	1076	1179	740	740
query47	2306	2433	2253	2253
query48	394	401	294	294
query49	627	471	352	352
query50	988	353	247	247
query51	4334	4257	4249	4249
query52	92	90	77	77
query53	251	264	193	193
query54	262	223	211	211
query55	79	76	68	68
query56	234	238	234	234
query57	1464	1395	1335	1335
query58	243	212	194	194
query59	1556	1637	1404	1404
query60	281	249	229	229
query61	163	181	157	157
query62	694	665	593	593
query63	228	184	186	184
query64	2536	833	672	672
query65 query66	1794	481	357	357
query67	29784	29726	29560	29560
query68 query69	414	305	270	270
query70	958	938	931	931
query71	296	233	213	213
query72	2947	2640	2380	2380
query73	847	770	444	444
query74	5115	4935	4793	4793
query75	2656	2621	2257	2257
query76	2323	1156	746	746
query77	362	368	284	284
query78	12395	12409	11728	11728
query79	1395	1057	766	766
query80	567	476	396	396
query81	447	274	247	247
query82	577	155	120	120
query83	362	269	249	249
query84	303	136	111	111
query85	879	537	425	425
query86	366	292	271	271
query87	3372	3334	3189	3189
query88	3578	2720	2722	2720
query89	425	378	327	327
query90	1931	175	179	175
query91	176	161	130	130
query92	65	60	57	57
query93	1464	1375	912	912
query94	558	345	309	309
query95	668	389	354	354
query96	1062	863	332	332
query97	2715	2710	2574	2574
query98	210	203	207	203
query99	1155	1155	1042	1042
Total cold run time: 250329 ms
Total hot run time: 169466 ms

sollhui · 2026-06-03T02:24:42Z

/review

github-actions

Review result: request changes.

I found one correctness issue in the path-style preservation logic: the change preserves only the canonical use_path_style key, but still overrides the supported s3.path-style-access alias by injecting use_path_style=false. This affects COS, OBS, and OSS provider/translation paths.

Critical checkpoint conclusions:

Goal/test: The PR aims to preserve explicit path-style configuration for OSS/COS/OBS. It covers canonical use_path_style=true, but not the existing s3.path-style-access alias, so the goal is incomplete.
Scope/focus: The code change is small and focused.
Concurrency/lifecycle/config/persistence/transactions/data writes: Not applicable to this filesystem property mapping change.
Parallel paths: The same provider/toS3Props pattern exists for COS, OBS, and OSS and needs the same fix in all affected paths.
Test coverage: Tests cover canonical use_path_style but miss the alias regression; please add alias coverage for provider-created storage and/or toS3Props.
Observability/performance: No additional observability or performance concerns found for this change.

User focus: No additional user-provided review focus was specified.

sollhui · 2026-06-03T02:43:54Z

run buildall

sollhui · 2026-06-03T02:44:02Z

/review

github-actions

No blocking issues found in the current PR diff.

Critical checkpoint conclusions:

Goal/test coverage: The PR preserves explicit path-style settings for COS/OBS/OSS provider creation and toS3Props conversion, including the s3.path-style-access alias. Added unit tests cover direct use_path_style, alias handling, and provider-created S3ObjStorage behavior for all three providers.
Scope/focus: The implementation is small and focused on avoiding insertion of the default use_path_style=false when either supported path-style key is already present.
Concurrency/lifecycle: No new shared mutable state, lifecycle management, or lock behavior is introduced.
Configuration/compatibility: No new config item or storage/protocol format is introduced. Existing default behavior remains false when neither path-style key is provided, while explicit caller values are preserved.
Parallel paths: Checked COS, OBS, and OSS provider paths and their toS3Props helpers. The previously raised alias concern is addressed in this head and was not duplicated.
Security/threat-model: Object storage endpoints and credentials remain admin-provided external connection properties under the repository threat model; this change does not add a new trust boundary or broaden access.
Observability/performance: No additional observability is needed for this config-preservation path; performance impact is negligible.

User focus: No additional user-provided review focus was specified.

Testing: Not run locally because thirdparty/installed/bin/protoc is missing, and FE instructions require that prerequisite before FE builds/tests.

hello-stephen · 2026-06-03T04:33:39Z

TPC-H: Total hot run time: 29609 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit ea84c38f79df12ed88b950d022643d6119c87f5d, data reload: false
------ Round 1 ----------------------------------
orders	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL ============================================
q1	17764	4086	4090	4086
q2	q3	10826	1494	852	852
q4	4687	491	342	342
q5	7610	890	585	585
q6	189	183	142	142
q7	803	839	638	638
q8	9367	1695	1637	1637
q9	6009	4538	4558	4538
q10	6774	1841	1524	1524
q11	436	281	251	251
q12	645	436	297	297
q13	18182	3435	2802	2802
q14	279	269	243	243
q15	q16	832	775	718	718
q17	998	910	955	910
q18	6951	5750	5520	5520
q19	1223	1310	1129	1129
q20	509	403	285	285
q21	6262	2912	2760	2760
q22	472	371	350	350
Total cold run time: 100818 ms
Total hot run time: 29609 ms
----- Round 2, with runtime_filter_mode=off -----
orders	Doris	NULL	NULL	150000000	42	6422171781	NULL	22778155	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL ============================================
q1	5198	4916	4848	4848
q2	q3	4891	5303	4682	4682
q4	2194	2201	1410	1410
q5	4952	4735	4783	4735
q6	236	182	134	134
q7	1867	1816	1517	1517
q8	2453	2168	2212	2168
q9	7876	7473	7478	7473
q10	4770	4695	4262	4262
q11	541	389	364	364
q12	737	735	529	529
q13	3015	3430	2815	2815
q14	285	278	260	260
q15	q16	682	702	623	623
q17	1309	1294	1283	1283
q18	7483	6900	6763	6763
q19	1115	1115	1126	1115
q20	2224	2234	1931	1931
q21	5302	4667	4526	4526
q22	528	470	416	416
Total cold run time: 57658 ms
Total hot run time: 51854 ms

hello-stephen · 2026-06-03T04:44:33Z

TPC-DS: Total hot run time: 170013 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 ea84c38f79df12ed88b950d022643d6119c87f5d, data reload: false
query5	4342	645	495	495
query6	463	204	185	185
query7	4841	558	315	315
query8	370	212	201	201
query9	8750	4085	4088	4085
query10	453	326	263	263
query11	5924	2370	2193	2193
query12	157	102	100	100
query13	1293	613	417	417
query14	6448	5471	5085	5085
query14_1	4488	4461	4442	4442
query15	214	201	182	182
query16	1066	494	422	422
query17	1173	729	610	610
query18	2703	500	354	354
query19	210	187	157	157
query20	115	113	104	104
query21	220	143	123	123
query22	13791	13639	13327	13327
query23	17428	16543	16261	16261
query23_1	16307	16370	16351	16351
query24	7426	1796	1338	1338
query24_1	1292	1310	1327	1310
query25	552	477	383	383
query26	1308	311	167	167
query27	2668	562	344	344
query28	4462	2032	2041	2032
query29	1075	605	487	487
query30	315	235	205	205
query31	1130	1084	957	957
query32	110	65	59	59
query33	540	337	283	283
query34	1192	1164	643	643
query35	748	773	675	675
query36	1391	1354	1267	1267
query37	150	104	92	92
query38	3232	3157	3082	3082
query39	937	912	907	907
query39_1	884	881	865	865
query40	216	130	104	104
query41	66	64	61	61
query42	95	94	95	94
query43	319	323	287	287
query44 query45	198	190	184	184
query46	1105	1252	770	770
query47	2311	2311	2195	2195
query48	394	412	304	304
query49	633	478	363	363
query50	968	355	262	262
query51	4324	4538	4300	4300
query52	89	94	77	77
query53	255	283	200	200
query54	267	219	201	201
query55	81	74	75	74
query56	238	225	220	220
query57	1440	1391	1303	1303
query58	252	215	212	212
query59	1576	1666	1455	1455
query60	280	255	233	233
query61	160	158	157	157
query62	696	656	605	605
query63	228	191	186	186
query64	2558	791	626	626
query65 query66	1785	475	343	343
query67	29744	29652	29680	29652
query68 query69	426	296	267	267
query70	986	956	968	956
query71	337	223	218	218
query72	2954	2740	2417	2417
query73	868	775	423	423
query74	5170	4969	4773	4773
query75	2663	2600	2248	2248
query76	2307	1148	758	758
query77	366	381	280	280
query78	12668	12541	11961	11961
query79	1287	1059	801	801
query80	530	469	395	395
query81	449	285	256	256
query82	231	159	120	120
query83	275	283	255	255
query84	250	148	116	116
query85	846	593	444	444
query86	332	324	296	296
query87	3376	3351	3185	3185
query88	3714	2745	2744	2744
query89	406	389	327	327
query90	2191	185	187	185
query91	195	172	139	139
query92	62	62	59	59
query93	1463	1454	871	871
query94	570	344	329	329
query95	698	479	370	370
query96	1059	768	346	346
query97	2665	2687	2559	2559
query98	211	205	204	204
query99	1159	1157	1058	1058
Total cold run time: 251595 ms
Total hot run time: 170013 ms

hello-stephen · 2026-06-03T07:09:40Z

FE Regression Coverage Report

Increment line coverage 0.00% (0/158) 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-06-03T08:20:55Z

FE Regression Coverage Report

Increment line coverage 0.00% (0/158) 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-06-03T09:07:10Z

FE Regression Coverage Report

Increment line coverage 0.00% (0/158) 🎉
Increment coverage report
Complete coverage report

hello-stephen · 2026-06-03T11:01:58Z

FE Regression Coverage Report

Increment line coverage 0.00% (0/895) 🎉
Increment coverage report
Complete coverage report

liaoxin01

LGTM

github-actions · 2026-06-03T18:06:59Z

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

CalvinKirs · 2026-06-04T01:53:33Z

#63400 This was already fixed in #63400

sollhui · 2026-06-04T02:23:44Z

#63400 This was already fixed in #63400

Get it.

sollhui requested review from CalvinKirs, gavinchou and morningman as code owners June 2, 2026 14:50

sollhui changed the title ~~[fix](fe) preserve explicit path style for object storage~~[fix](s3) preserve explicit path style for object storageJun 2, 2026

sollhui force-pushed the fix_preserve_use_path_style branch from bd3c424 to fdf08ceCompare June 2, 2026 15:16

github-actionsBot requested changes Jun 3, 2026
View reviewed changes

Comment thread...m/fe-filesystem-cos/src/main/java/org/apache/doris/filesystem/cos/CosFileSystemProvider.java Outdated

preserve explicit path style for object storage
ea84c38

sollhui force-pushed the fix_preserve_use_path_style branch from fdf08ce to ea84c38Compare June 3, 2026 02:39

github-actionsBot reviewed Jun 3, 2026
View reviewed changes

liaoxin01 approved these changes Jun 3, 2026
View reviewed changes

github-actionsBot added the approved Indicates a PR has been approved by one committer. label Jun 3, 2026

sollhui closed this Jun 4, 2026

Uh oh!

Conversation

sollhui commented Jun 2, 2026

What problem does this PR solve?

Uh oh!

sollhui commented Jun 2, 2026

Uh oh!

sollhui commented Jun 2, 2026

Uh oh!

hello-stephen commented Jun 2, 2026

FE UT Coverage Report

Uh oh!

hello-stephen commented Jun 2, 2026

Uh oh!

hello-stephen commented Jun 2, 2026

FE UT Coverage Report

Uh oh!

hello-stephen commented Jun 2, 2026

Uh oh!

sollhui commented Jun 3, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sollhui commented Jun 3, 2026

Uh oh!

sollhui commented Jun 3, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

hello-stephen commented Jun 3, 2026

Uh oh!

hello-stephen commented Jun 3, 2026

Uh oh!

hello-stephen commented Jun 3, 2026

FE Regression Coverage Report

Uh oh!

hello-stephen commented Jun 3, 2026

FE Regression Coverage Report

Uh oh!

hello-stephen commented Jun 3, 2026

FE Regression Coverage Report

Uh oh!

hello-stephen commented Jun 3, 2026

FE Regression Coverage Report

Uh oh!

liaoxin01 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actionsBot commented Jun 3, 2026

Uh oh!

CalvinKirs commented Jun 4, 2026

Uh oh!

sollhui commented Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants