Skip to content

[fix](iceberg) Validate Parquet compression before file creation - #69376

Merged
Gabriel39 merged 5 commits into
apache:masterfrom
hubgeter:fix/iceberg-validate-parquet-compression-master
Nov 5, 2026
Merged

Gabriel39 merged 5 commits into
apache:masterfrom
hubgeter:fix/iceberg-validate-parquet-compression-master

Conversation

@hubgeter

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: N/A

Related PR: #68854, #69338

Problem Summary:
The Iceberg data, position-delete, and Hive Parquet writers created their output file before validating the requested Parquet compression codec. For an unsupported codec, the Iceberg data writer then attempted to delete an open file, while the other writers returned after creating the file. This made error cleanup depend on filesystem behavior and could leave an orphan file. Resolve the codec before calling create_file so invalid codecs fail without creating any output file, and add a regression test for the failure path.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test: added UnsupportedParquetCodecDoesNotCreateFile; CI will run it.
    • 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

@hello-stephen

Copy link
Copy Markdown
Contributor

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?

hubgeter added a commit to hubgeter/doris that referenced this pull request Oct 21, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#66348, apache#68854, apache#69376

Problem Summary:
Partition writers call open before registering a new writer in the partition map. If the output file is created but the Parquet or ORC transformer then fails to open, the table writer cannot reach that writer through its close path. Clean up the created file at the open failure boundary so the error cannot leave an orphan, matching the lifecycle hardening introduced on master by apache#66348.

### Release note

None

### Check List (For Author)

- Test
    - [ ] Regression test
    - [ ] Unit Test
    - [x] Manual test: repository clang-format check and direct compilation of the modified BE writer translation unit passed.
- Behavior changed:
    - [x] No.
- Does this need documentation?
    - [x] No.
yiguolei
yiguolei previously approved these changes Oct 21, 2026
@yiguolei

Copy link
Copy Markdown
Contributor

run buildall

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17589	5112	5041	5041
q2	2115	309	254	254
q3	10245	1343	798	798
q4	4684	460	341	341
q5	7431	786	536	536
q6	182	183	151	151
q7	742	805	597	597
q8	9253	1311	1025	1025
q9	5450	4169	4150	4150
q10	6814	1287	1020	1020
q11	435	251	229	229
q12	633	394	286	286
q13	18105	3527	2999	2999
q14	274	263	245	245
q15	q16	725	709	670	670
q17	1178	744	575	575
q18	7002	6174	6093	6093
q19	1105	950	600	600
q20	381	332	225	225
q21	4946	2099	2275	2099
q22	341	294	248	248
Total cold run time: 99630 ms
Total hot run time: 28182 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5629	5630	5601	5601
q2	671	582	541	541
q3	5209	5389	4916	4916
q4	1988	2119	1546	1546
q5	5329	5192	5490	5192
q6	253	207	159	159
q7	2390	1999	1903	1903
q8	2978	2658	2706	2658
q9	8005	8063	7920	7920
q10	3748	3664	3224	3224
q11	611	424	557	424
q12	724	705	526	526
q13	3587	3839	3292	3292
q14	302	306	294	294
q15	q16	703	743	659	659
q17	8190	7306	6894	6894
q18	12996	12215	12870	12215
q19	864	763	763	763
q20	2157	2156	1908	1908
q21	5568	4555	4858	4555
q22	499	467	393	393
Total cold run time: 72401 ms
Total hot run time: 65583 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150921 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 cf149580fc0c66f1e63c0ee88bcb7a4f96b320e0, data reload: false

query5	4307	586	431	431
query6	424	207	190	190
query7	4798	446	234	234
query8	317	161	151	151
query9	8765	3053	3065	3053
query10	479	311	262	262
query11	6031	3848	3590	3590
query12	148	92	89	89
query13	1273	445	339	339
query14	6059	3935	3676	3676
query14_1	3389	3410	3464	3410
query15	212	199	182	182
query16	943	438	415	415
query17	876	665	540	540
query18	2421	440	320	320
query19	197	167	136	136
query20	85	84	87	84
query21	215	129	107	107
query22	12941	12984	12817	12817
query23	13443	12965	12472	12472
query23_1	12557	12527	12545	12527
query24	7365	817	459	459
query24_1	465	468	465	465
query25	513	389	341	341
query26	1253	255	142	142
query27	2786	440	266	266
query28	4617	1902	1913	1902
query29	1533	600	450	450
query30	303	221	185	185
query31	1009	926	815	815
query32	137	87	93	87
query33	509	286	229	229
query34	891	847	494	494
query35	736	764	641	641
query36	831	850	728	728
query37	123	103	79	79
query38	1779	1731	1644	1644
query39	787	809	739	739
query39_1	734	739	733	733
query40	218	112	93	93
query41	76	64	62	62
query42	86	86	83	83
query43	345	370	312	312
query44	1331	681	681	681
query45	182	178	163	163
query46	837	980	581	581
query47	3055	3049	2707	2707
query48	304	311	238	238
query49	593	414	288	288
query50	716	279	211	211
query51	10404	10377	10339	10339
query52	81	80	69	69
query53	187	202	148	148
query54	227	199	173	173
query55	72	66	64	64
query56	231	222	211	211
query57	1658	1618	1574	1574
query58	295	268	269	268
query59	2123	2157	1959	1959
query60	313	256	225	225
query61	172	173	167	167
query62	397	340	280	280
query63	186	165	163	163
query64	2873	1070	861	861
query65	3359	3275	3304	3275
query66	1841	429	320	320
query67	20327	20227	20084	20084
query68	3333	990	606	606
query69	403	280	267	267
query70	861	779	759	759
query71	300	239	216	216
query72	2873	2641	2234	2234
query73	520	528	305	305
query74	4651	4538	4396	4396
query75	2353	2291	1911	1911
query76	2417	1019	595	595
query77	341	356	264	264
query78	9839	9802	9193	9193
query79	969	847	516	516
query80	736	443	338	338
query81	576	321	268	268
query82	578	141	108	108
query83	305	196	185	185
query84	300	119	101	101
query85	867	428	375	375
query86	350	258	237	237
query87	2007	2036	1807	1807
query88	3694	2655	2686	2655
query89	332	278	244	244
query90	1798	176	166	166
query91	164	149	133	133
query92	103	104	81	81
query93	1002	1027	569	569
query94	515	311	270	270
query95	585	324	374	324
query96	683	498	232	232
query97	2431	2423	2292	2292
query98	158	154	150	150
query99	783	756	652	652
Total cold run time: 235300 ms
Total hot run time: 150921 ms

@hello-stephen

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

query1	0.01	0.01	0.01
query2	0.09	0.05	0.05
query3	0.26	0.10	0.09
query4	1.65	0.10	0.11
query5	0.22	0.23	0.24
query6	1.18	0.69	0.68
query7	0.03	0.01	0.01
query8	0.05	0.03	0.03
query9	0.40	0.34	0.34
query10	0.60	0.57	0.56
query11	0.21	0.15	0.14
query12	0.18	0.15	0.14
query13	0.51	0.50	0.50
query14	1.05	1.01	1.02
query15	0.64	0.60	0.60
query16	0.36	0.34	0.34
query17	1.12	1.12	1.14
query18	0.19	0.20	0.19
query19	2.14	1.99	2.01
query20	0.02	0.01	0.01
query21	15.48	0.18	0.13
query22	4.92	0.05	0.06
query23	16.29	0.32	0.12
query24	3.06	0.61	0.34
query25	0.11	0.05	0.04
query26	1.49	0.22	0.15
query27	0.04	0.04	0.03
query28	3.38	0.75	0.38
query29	12.51	4.22	3.27
query30	0.27	0.14	0.10
query31	2.77	0.55	0.32
query32	3.22	0.59	0.50
query33	3.10	3.23	3.25
query34	15.37	4.03	3.39
query35	3.35	3.34	3.36
query36	0.56	0.48	0.42
query37	0.09	0.06	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.15	0.15
query41	0.08	0.03	0.02
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 97.34 s
Total hot run time: 24.31 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 66.46% (31869/47954)
Line Coverage 51.24% (335746/655290)
Region Coverage 46.62% (271530/582480)
Branch Coverage 48.37% (128127/264916)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.12% (35834/46463)
Line Coverage 62.37% (404340/648262)
Region Coverage 58.83% (341403/580360)
Branch Coverage 59.80% (158029/264259)

@yiguolei

Copy link
Copy Markdown
Contributor

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed all four changed files at cf14958. No new actionable defect was substantiated, and there are no existing inline blockers. Two complete-scope reviewers and a separate failure-lifecycle reviewer each returned NO_NEW_VALUABLE_FINDINGS on the unchanged empty finding set. Static review converged after one round; no unresolved code-path question remains within the assigned scope.

Critical checkpoint conclusions:

  1. Goal and evidence: all three Parquet writers resolve compression before create_file. Unsupported codecs return the original error without constructing a file writer or transformer. The added test checks rejection and file absence, with the limitation below.
  2. Scope and reuse: the change is focused and retains the shared codec conversion helper; each local lambda preserves the existing creation arguments.
  3. Concurrency and locking: existing async writer ownership, sort mutexes, and active-writer snapshots remain unchanged. Failed opens are excluded from newly published snapshots; no new thread, lock, shared state, or lock ordering is introduced.
  4. Lifecycle and memory safety: traced static/dynamic partition dispatch, sort, MERGE, delete-sink cleanup, and destruction. Async failure reaches close; null transformers are guarded, failed writers cannot build commits, and original failure is preserved. No new static initialization or ownership cycle.
  5. Configuration: none added or changed.
  6. Compatibility: no interface, successful codec encoding, or storage-format change; no new rolling-upgrade requirement.
  7. Thrift/protobuf: no definitions or field IDs changed.
  8. Parallel paths: Iceberg data/delete Parquet and ORC, plus Hive Parquet/ORC/CSV, each retain exactly one creation call on supported branches. Hive retains its S3 committer option. GZ still maps to GZIP.
  9. Conditional/error handling: DORIS_TRY and RETURN_IF_ERROR propagate codec and creation failures. Existing transformer-error handling remains; no speculative conditional check was added.
  10. Test coverage: the new local Iceberg negative test is registered by the existing recursive test glob and uses supported BE-UT private access. No new Hive/delete, successful-codec, or remote-filesystem case was added. Builds and tests were not run under the static-only contract.
  11. Test expectations/results: the expected unsupported-codec error and eventual absence match the inspected logic; no golden result files changed.
  12. Observability: codec errors and existing cleanup warnings remain available; this ordering change needs no additional metrics or logging.
  13. Persistence/failover: no EditLog, metadata replay, or commit-protocol change. Rejected opens cannot reach commit publication along the inspected callers.
  14. Writes/atomicity/crashes: successful write/report behavior is preserved; rejected compression avoids the unnecessary output-file side effect. No new crash leak or publication inconsistency was identified.
  15. FE/BE variables: none added or changed.
  16. Performance: the immediate stack lambda adds no allocation or scan, while rejected codecs avoid file creation. Existing memory accounting is unaffected; no benchmark was run.
  17. Other concerns: rechecked format defaults, rollover, partial initialization, lambda lifetime, options, and cleanup. No additional issue was substantiated. No extra user focus was supplied.

Validation limit: UnsupportedParquetCodecDoesNotCreateFile asserts eventual file absence. On the local filesystem, the previous create-then-delete implementation also satisfies that assertion, so the test alone does not prove that creation was never attempted. An assertion that _file_writer remains null or instrumentation of create_file would strengthen that invariant. This is a test-strength observation; static control-flow inspection confirms the production ordering fix.

Review converged after 1 round(s).

Gabriel39
Gabriel39 previously approved these changes Oct 22, 2026
@Gabriel39

Copy link
Copy Markdown
Contributor

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed all four changed paths at cf14958. No new verified defects or existing blocking inline findings. The main review and both assigned reviewers converged after one round; both reviewers returned NO_NEW_VALUABLE_FINDINGS. No additional user focus was supplied.

Critical checkpoint conclusions:

  1. Goal and correctness: Iceberg data, Iceberg position-delete, and Hive Parquet writers resolve the shared compression mapping before create_file. Unsupported codec errors propagate before writing or constructing commit metadata. The added negative unit test checks the returned error and absence of a residual file, subject to item 10.
  2. Scope and reuse: The change is small and focused. Local synchronous lambdas reuse the existing file path/options, and the shared Parquet codec helper remains authoritative.
  3. Concurrency and locks: No new shared state, threads, locks, or lock ordering. The existing async writer preserves failures through close; sorting and rollover retain their existing synchronization.
  4. Lifecycle and static initialization: Lambda captures remain alive throughout the synchronous call. Failed codec validation leaves no file writer or transformer; retained failed writers receive close(error), whose null checks and status guards prevent publication. Dynamic failed writers are destroyed locally. No new static initialization or ownership cycle.
  5. Configuration: No configuration items or dynamic-update behavior changed.
  6. Compatibility: No public signature, compression mapping, persisted file encoding, or protocol changed. Hive file options and compatibility gate remain intact. No new libc API was introduced; completed ELF artifacts and runtime compatibility were not audited.
  7. Thrift/protobuf: No definitions, field IDs, enum values, or required fields changed.
  8. Parallel paths: All three Parquet branches were checked. Iceberg data/delete ORC and Hive ORC/CSV still create exactly one file before constructing their transformer; unsupported formats do not create one. Initial open, static/unpartitioned versus dynamic registration, and sorted rollover were traced.
  9. Conditions and error handling: Standard DORIS_TRY/RETURN_IF_ERROR propagation preserves non-OK failures. No new conditional predicate was introduced; existing post-creation transformer cleanup remains.
  10. Test coverage: The new test is valid by static inspection, uses an isolated UUID path, and is included by the BE unit-test glob. Its existence assertion does not prove that create_file was skipped: the previous Iceberg implementation could create and then unlink the file before returning, also satisfying that assertion on Linux. A create-call assertion or checking that _file_writer remains null would test the ordering directly. No new Hive/delete-writer-specific or end-to-end tests are added. This is a confirmed validation limitation, with no unresolved production-code candidate.
  11. Expected results: No regression .out or other expected-result files changed. The error substring matches the shared helper; the filesystem absence check matches the current final state.
  12. Observability: Existing returned codec errors, cleanup warnings, and writer profiles are adequate for this ordering change; no additional logging or metrics are needed.
  13. Transactions, persistence, and failover: No EditLog, replay, transaction protocol, or persisted metadata changed. Existing success guards still control Iceberg/Hive commit updates and delete-file commit data.
  14. Writes and atomicity: Invalid codecs avoid the file side effect. Actual caller failure propagation prevents a failed open from publishing data, and supported-format create/transformer failure cleanup is preserved. No new crash-leak window was identified.
  15. FE/BE variable passing: No new transmitted variables or sender paths.
  16. Performance and memory: Successful writes retain one conversion and one create; the local lambda adds no owning allocation. Invalid codecs avoid filesystem creation and file-writer allocation. No new large allocation, memory-tracking, reservation, or column/null-handling behavior.
  17. Final recheck: Revisited all changed paths, codec success/failure, failed creation, null-transformer cleanup, sort rollover, unsupported formats, and delete-sink publication. No additional verified defect or unresolved review question remains. Visibility/version and MoW delete-bitmap rules are unaffected.

This was a static review: no builds, tests, binary audits, or source modifications were performed. Convergence covers the reviewed source and explicit candidate dispositions; it does not establish runtime test success.

Review converged after 1 round(s).

@hubgeter
hubgeter dismissed stale reviews from Gabriel39 and yiguolei via b47b8d1 October 22, 2026 03:32
@hubgeter
hubgeter force-pushed the fix/iceberg-validate-parquet-compression-master branch from cf14958 to b47b8d1 Compare October 22, 2026 03:32
@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@hubgeter

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed all four changed paths at b47b8d1. No new actionable correctness findings, and no existing inline P0/P1 findings to carry forward. Two general reviewers and a separate lifecycle reviewer checked the unchanged empty inline set.

Critical checkpoint conclusions:

  1. Goal: all three writers resolve the Parquet codec before creating the output file; unsupported codecs return the existing error without allocating a file writer.
  2. Scope: focused changes to creation ordering, plus one negative unit test; the existing filesystem API and shared codec mapping are reused.
  3. Concurrency: the new lambdas execute synchronously within open and retain no stack references beyond it; no changed threads, locks, atomics, or dependency signaling.
  4. Lifecycle: checked static, dynamic, unpartitioned, sorted, rollover, and delete-sink callers. Open failures propagate into failure cleanup; no newly reachable successful commit or null-transformer use was found. No new static initialization.
  5. Configuration: none added or changed.
  6. Compatibility: supported codec mappings, filesystem options, transformer arguments, timestamp settings, and file encodings remain unchanged; no signature or new libc API dependency.
  7. Thrift/protobuf: no definition or field-ID changes.
  8. Parallel paths: all three generic-to-Parquet codec callers are updated; Iceberg ORC and Hive ORC/text retain their creation flow.
  9. Conditions and errors: codec and filesystem errors remain checked and propagated; unsupported format branches avoid creation. No newly ignored Status.
  10. Test coverage: the added test checks ZLIB rejection and final local-file absence. Its assertions also hold for the previous create-then-delete data-writer implementation, so it does not prove zero calls to create_file. Intercepting/counting that operation would strengthen the regression check. The other two writers and remote-storage failure behavior have no added targeted test.
  11. Test results: no expected-result files changed. No builds, unit tests, regression tests, formatting, or static-analysis commands were run under this static-only review contract.
  12. Observability: existing codec, format, and filesystem errors remain available; no additional logging or metrics appear necessary for this ordering fix.
  13. Transactions and persistence: no EditLog, metadata replay, visible-version, or MoW changes; commit reporting and rollback ownership remain unchanged.
  14. Data writes and atomicity: rejected codecs now fail before output creation; supported write/close/report paths and existing upload cleanup are preserved. No new crash-cleanup regression was identified by code tracing.
  15. FE/BE variables: no transmitted variables or send sites changed.
  16. Performance and memory: no new hot-loop work, large allocation, ownership cycle, or retry; invalid codecs avoid file creation and cleanup.
  17. Further recheck and focus: all changed hunks and special cases were rechecked; no additional user focus was supplied and no unresolved correctness candidates remain.

Validation limits: conclusions are based on static code inspection. Compile/link, test execution, and object-store behavior were not verified at runtime. The test limitations above are nonblocking coverage observations, not separate inline bug findings.

Review converged after 1 round(s).

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17578	5146	5024	5024
q2	2080	297	248	248
q3	10294	1355	809	809
q4	4678	456	335	335
q5	7434	781	532	532
q6	183	185	156	156
q7	780	784	603	603
q8	9269	1327	1082	1082
q9	5392	4193	4209	4193
q10	6832	1296	1027	1027
q11	437	254	237	237
q12	635	396	279	279
q13	18068	3506	3014	3014
q14	272	276	250	250
q15	q16	718	713	634	634
q17	1193	744	577	577
q18	7063	6172	6095	6095
q19	1102	954	613	613
q20	378	328	227	227
q21	4356	2122	2291	2122
q22	340	292	255	255
Total cold run time: 99082 ms
Total hot run time: 28312 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5601	5567	5575	5567
q2	668	584	545	545
q3	5182	5381	4936	4936
q4	1990	2116	1535	1535
q5	5244	5152	5353	5152
q6	250	201	155	155
q7	2268	2060	1855	1855
q8	3065	2663	2737	2663
q9	8110	7916	7997	7916
q10	3716	3638	3268	3268
q11	613	435	418	418
q12	841	729	538	538
q13	3481	3943	3285	3285
q14	304	314	287	287
q15	q16	709	733	643	643
q17	8304	7233	6899	6899
q18	13022	12150	12890	12150
q19	895	769	760	760
q20	2127	2141	1921	1921
q21	5597	4559	4653	4559
q22	496	451	410	410
Total cold run time: 72483 ms
Total hot run time: 65462 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150389 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 b47b8d1f73065928a4ceb98375a5643f02cce6f4, data reload: false

query5	4307	593	440	440
query6	426	213	193	193
query7	4801	445	221	221
query8	318	184	151	151
query9	8757	3072	3070	3070
query10	452	307	257	257
query11	5995	3863	3575	3575
query12	144	96	84	84
query13	1259	476	335	335
query14	6074	3893	3583	3583
query14_1	3403	3444	3368	3368
query15	207	206	183	183
query16	942	428	385	385
query17	863	645	546	546
query18	2412	414	302	302
query19	194	158	127	127
query20	83	81	87	81
query21	206	126	104	104
query22	12958	12956	12860	12860
query23	13539	12896	12534	12534
query23_1	12436	12622	12388	12388
query24	7352	802	445	445
query24_1	455	460	453	453
query25	512	397	341	341
query26	1261	257	138	138
query27	2760	434	261	261
query28	4629	1898	1889	1889
query29	1543	567	408	408
query30	291	212	182	182
query31	994	925	819	819
query32	143	85	91	85
query33	521	285	232	232
query34	884	856	519	519
query35	734	752	656	656
query36	819	810	724	724
query37	118	94	79	79
query38	1777	1837	1711	1711
query39	795	799	765	765
query39_1	746	734	748	734
query40	221	121	94	94
query41	68	63	64	63
query42	88	116	90	90
query43	349	372	315	315
query44	1328	681	686	681
query45	179	174	169	169
query46	849	967	572	572
query47	2989	3005	2678	2678
query48	294	302	209	209
query49	570	395	287	287
query50	652	272	210	210
query51	10508	10264	10475	10264
query52	76	83	73	73
query53	182	211	150	150
query54	229	186	182	182
query55	72	69	64	64
query56	237	218	199	199
query57	1655	1628	1584	1584
query58	282	265	254	254
query59	2155	2115	1941	1941
query60	289	249	237	237
query61	143	145	148	145
query62	383	341	284	284
query63	187	155	159	155
query64	2715	907	830	830
query65	3324	3290	3268	3268
query66	1794	456	326	326
query67	19999	19946	19946	19946
query68	3006	975	614	614
query69	391	291	244	244
query70	861	777	780	777
query71	306	238	226	226
query72	2765	2643	2379	2379
query73	509	525	295	295
query74	4654	4537	4357	4357
query75	2327	2294	1966	1966
query76	2321	1004	615	615
query77	317	360	261	261
query78	9748	9726	9145	9145
query79	973	883	513	513
query80	786	413	320	320
query81	575	324	268	268
query82	573	129	107	107
query83	277	197	180	180
query84	313	126	99	99
query85	904	410	362	362
query86	341	231	230	230
query87	2001	1973	1815	1815
query88	3596	2647	2628	2628
query89	331	284	233	233
query90	1743	162	163	162
query91	155	147	120	120
query92	97	90	86	86
query93	997	984	566	566
query94	489	299	282	282
query95	577	384	297	297
query96	651	477	231	231
query97	2437	2449	2295	2295
query98	160	151	150	150
query99	773	773	652	652
Total cold run time: 233622 ms
Total hot run time: 150389 ms

@hello-stephen

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

query1	0.01	0.01	0.01
query2	0.10	0.05	0.05
query3	0.26	0.09	0.09
query4	1.61	0.08	0.10
query5	0.24	0.24	0.23
query6	1.16	0.70	0.68
query7	0.04	0.00	0.00
query8	0.05	0.04	0.04
query9	0.40	0.33	0.34
query10	0.61	0.62	0.56
query11	0.20	0.15	0.14
query12	0.19	0.14	0.15
query13	0.49	0.48	0.49
query14	1.02	1.02	1.02
query15	0.63	0.60	0.62
query16	0.35	0.34	0.34
query17	1.16	1.16	1.09
query18	0.21	0.20	0.20
query19	2.04	1.94	1.97
query20	0.02	0.02	0.01
query21	15.48	0.18	0.14
query22	4.90	0.04	0.05
query23	16.27	0.32	0.13
query24	2.90	0.62	0.34
query25	0.12	0.07	0.04
query26	1.20	0.21	0.16
query27	0.04	0.04	0.06
query28	3.21	0.76	0.36
query29	12.49	4.14	3.24
query30	0.28	0.13	0.10
query31	2.77	0.55	0.32
query32	3.23	0.60	0.51
query33	3.15	3.19	3.18
query34	15.24	3.98	3.37
query35	3.37	3.31	3.33
query36	0.56	0.44	0.41
query37	0.10	0.06	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.14	0.15
query41	0.08	0.04	0.03
query42	0.04	0.02	0.03
query43	0.04	0.04	0.03
Total cold run time: 96.52 s
Total hot run time: 24.07 s

@hubgeter
hubgeter force-pushed the fix/iceberg-validate-parquet-compression-master branch from b47b8d1 to ccc5227 Compare October 27, 2026 02:20
@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@hubgeter

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Static review of PR #69376 at ccc5227 identified one P2 regression-test defect, reported inline. The production changes put unsupported Parquet codec rejection before file creation in all three writers; no introduced production correctness defect was found. There are no existing inline threads or independently confirmed existing P0/P1 blockers, and no additional user-provided focus points.

Critical checkpoint conclusions:

  1. Goal: the source achieves reject-before-create. The new test does not establish that property because the old create-and-delete flow satisfies its assertions; see the inline finding.
  2. Scope and clarity: the changes are small and focused; supported transformer options and writer options remain the same.
  3. Concurrency and locking: the local lambdas run synchronously and do not escape. No thread, shared variable, lock, lock order, or atomic ordering is added or changed; existing I/O dispatch waits for completion.
  4. Lifecycle and initialization: normal, spill, sorted, MERGE, Hive async, and delete owners preserve failed-open status before cleanup or commit/report. Null writer/transformer members on rejection are safe in those callers. No new ownership cycle or static initializer is introduced.
  5. Configuration: no configuration is added or changed; dynamic propagation is not applicable.
  6. Compatibility: no interface, wire format, storage encoding, compression mapping, or supported timestamp behavior changes. No new external API or toolchain dependency is introduced; no release-artifact ABI audit was run.
  7. Thrift/protobuf: no definitions, field IDs, or enum values change.
  8. Parallel paths: Iceberg data and delete Parquet/ORC plus Hive Parquet/ORC/text and unsupported-format branches were checked. Every supported branch still creates exactly one file writer before transformer open.
  9. Conditions and error handling: no new if/else predicate is added. Existing Result/Status macros preserve codec and creation errors, with no silent continuation into writes or commit construction.
  10. Test coverage: the added local negative test has the inline-reported blind spot. No new Hive/delete negative-codec tests are included. Related supported writer and cleanup tests were inspected statically; no build or test was executed.
  11. Expected results: no golden output files change. The unit-test expectations were reviewed from source; no executed result is claimed.
  12. Observability: unsupported-codec Status text and existing close/cleanup warnings are preserved. This ordering change does not add a stage needing new metrics or logs.
  13. Transactions and persistence: successful external commit/report gating and rollback protocols are unchanged. No EditLog, persistence schema, or failover protocol changes.
  14. Data writes and atomicity: rejected codecs create no writer/upload; successful formats retain their creation/transform/write/close sequence. Retained failed writers close with failure, preventing missing-file commits; dynamic failures destruct before insertion. Existing crash/rollback guarantees are unchanged. Internal visible versions and MoW delete bitmaps are not affected.
  15. FE/BE variables: none are added or changed.
  16. Performance and memory: rejected codecs avoid file I/O and writer allocation; supported paths retain one creation call. The reference-capturing local lambdas introduce no material buffer or reservation requirement.
  17. Other BE checks: no hub header, include, PCH, template-instantiation, unity-skip, or hygiene-table change occurs; the test includes no source .cpp. The final recheck found no additional verified issue.

Coverage includes all four changed files and relevant caller, cleanup, filesystem, transformer, and report paths. No unresolved static candidate remains. Validation is limited to static inspection: builds, unit/regression tests, remote-filesystem execution, and runtime experiments were not run, as required by the review contract.

Review converged after 2 round(s).

hubgeter added a commit to hubgeter/doris that referenced this pull request Oct 27, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69376

Problem Summary:
The negative codec test only checked that no file remained after open. The previous create-then-delete implementation could also satisfy that assertion. Instrument LocalFileSystem::create_file_impl through the existing sync point and assert it is never called, while retaining the null writer and final absence checks.

### Release note

None

### Check List (For Author)

- Test
    - [ ] Regression test
    - [x] Unit Test: strengthened UnsupportedParquetCodecDoesNotCreateFile; CI will run it.
    - [ ] Manual test
- Behavior changed:
    - [x] No.
- Does this need documentation?
    - [x] No.
@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@hubgeter

Copy link
Copy Markdown
Contributor Author

/review

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.23% (32071/47705)
Line Coverage 52.05% (337135/647703)
Region Coverage 47.59% (274067/575947)
Branch Coverage 49.42% (129155/261361)

@hello-stephen

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

query1	0.01	0.01	0.01
query2	0.09	0.05	0.05
query3	0.26	0.09	0.09
query4	1.60	0.10	0.10
query5	0.26	0.24	0.24
query6	1.17	0.67	0.68
query7	0.04	0.01	0.01
query8	0.05	0.04	0.04
query9	0.40	0.34	0.35
query10	0.60	0.61	0.58
query11	0.20	0.14	0.14
query12	0.18	0.14	0.15
query13	0.51	0.51	0.50
query14	1.06	1.01	1.01
query15	0.64	0.60	0.60
query16	0.32	0.35	0.34
query17	1.16	1.17	1.12
query18	0.22	0.20	0.20
query19	2.16	1.97	2.01
query20	0.02	0.01	0.01
query21	15.43	0.20	0.14
query22	4.86	0.06	0.05
query23	16.38	0.32	0.12
query24	2.70	0.51	0.40
query25	0.12	0.07	0.04
query26	1.62	0.22	0.16
query27	0.04	0.04	0.04
query28	3.73	0.80	0.36
query29	12.53	4.17	3.24
query30	0.28	0.15	0.11
query31	2.77	0.56	0.33
query32	3.22	0.59	0.51
query33	3.17	3.25	3.13
query34	15.33	4.04	3.40
query35	3.35	3.37	3.35
query36	0.57	0.45	0.44
query37	0.10	0.07	0.06
query38	0.06	0.04	0.03
query39	0.04	0.03	0.02
query40	0.19	0.14	0.15
query41	0.10	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.03	0.03
Total cold run time: 97.62 s
Total hot run time: 24.32 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 69.25% (32005/46217)
Line Coverage 53.94% (345548/640633)
Region Coverage 49.63% (284497/573255)
Branch Coverage 51.12% (133122/260415)

1 similar comment
@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 69.25% (32005/46217)
Line Coverage 53.94% (345548/640633)
Region Coverage 49.63% (284497/573255)
Branch Coverage 51.12% (133122/260415)

@hubgeter

hubgeter commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Local pipeline review — ✅ PASS

schema: doris-repo-review/v1
status: PASS
pr: apache/doris#69376
commit: bf2a3b312d64a4df17eceeebfd0c23b136ef2293
base: 7acfd9bfc27f01f0b953e8ac0cdc3482a46e3c59
reviewed_at: 2026-11-04T07:12+00:00
reviewer: hubgeter
model: gpt-6.1-sol
effort: max
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: true

Notes for maintainers

  • Static review only; compilation, BE unit tests, regression tests, and filesystem/cluster execution were not run.
  • be/test/exec/sink/writer/iceberg/iceberg_partition_writer_test.cpp:188 directly covers the local Iceberg data writer; Hive and position-delete codec-rejection ordering were inspected statically.

Reviewed locally with the doris-repo-review pipeline. Repository policy may accept this receipt for the matching commit; it is not a human Apache approval.

hubgeter added a commit to hubgeter/doris that referenced this pull request Nov 4, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69376

Problem Summary: The Iceberg data, position-delete and Hive partition writers
wrap a single filesystem create_file call in forwarding lambdas. Expand the
calls directly at each format branch to make the validation and file-creation
order explicit, while preserving every argument, writer option and error path.

### Release note

None

### Check List (For Author)

- Test: clang-format 16 via the repository formatter and git diff --check passed.
    - No need to rerun runtime tests: this mechanically expands seven existing
      forwarding calls without changing arguments or branch positions. Verified
      the remaining source tokens and existing negative test are unchanged.
- Behavior changed: No.
- Does this need documentation: No.
@hubgeter

hubgeter commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Local pipeline review — ✅ PASS

schema: doris-repo-review/v1
status: PASS
pr: apache/doris#69376
commit: 9b8cc64cf043ae1b6d3f53eab10c77cfe0c722ef
base: 7acfd9bfc27f01f0b953e8ac0cdc3482a46e3c59
reviewed_at: 2026-11-04T08:35+00:00
reviewer: hubgeter
model: gpt-6.1-sol
effort: max
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: true

Notes for maintainers

  • Static review only; compilation, BE unit tests, regression tests, and filesystem/cluster execution were not run.
  • Reused 8 content-reconfirmed dismissals from bf2a3b3; all 7 direct creation calls were checked for unchanged ordering, options, single evaluation, status and lifetime.
  • be/test/exec/sink/writer/iceberg/iceberg_partition_writer_test.cpp:188 directly covers the local Iceberg data writer; Hive and position-delete codec-rejection ordering were inspected statically.

Reviewed locally with the doris-repo-review pipeline. Repository policy may accept this receipt for the matching commit; it is not a human Apache approval.

@hubgeter

hubgeter commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17596	5194	5047	5047
q2	2148	303	263	263
q3	10230	1421	837	837
q4	4684	462	337	337
q5	7449	800	524	524
q6	189	182	155	155
q7	772	793	594	594
q8	9244	1401	1018	1018
q9	5468	4206	4181	4181
q10	6807	1304	1018	1018
q11	425	246	227	227
q12	647	398	280	280
q13	18109	3641	3035	3035
q14	282	273	256	256
q15	q16	760	716	663	663
q17	1201	752	577	577
q18	7047	6191	6125	6125
q19	1117	966	590	590
q20	384	336	230	230
q21	4980	2126	2366	2126
q22	359	326	270	270
Total cold run time: 99898 ms
Total hot run time: 28353 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5642	5612	5615	5612
q2	719	600	565	565
q3	5278	5397	4934	4934
q4	2002	2176	1563	1563
q5	5249	5162	5695	5162
q6	247	203	152	152
q7	2393	2131	1933	1933
q8	3119	2931	2743	2743
q9	7994	8116	8080	8080
q10	3771	3687	3264	3264
q11	617	456	432	432
q12	681	700	521	521
q13	3652	3908	3263	3263
q14	312	326	293	293
q15	q16	711	727	677	677
q17	8485	7677	7110	7110
q18	12931	12149	12974	12149
q19	938	828	814	814
q20	2151	2135	1920	1920
q21	5883	4901	4768	4768
q22	507	455	422	422
Total cold run time: 73282 ms
Total hot run time: 66377 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151062 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 9b8cc64cf043ae1b6d3f53eab10c77cfe0c722ef, data reload: false

query5	4304	579	438	438
query6	425	204	204	204
query7	4832	450	223	223
query8	319	185	152	152
query9	8787	3162	3115	3115
query10	489	318	254	254
query11	6017	3827	3561	3561
query12	143	91	87	87
query13	1251	458	331	331
query14	6183	3955	3647	3647
query14_1	3471	3473	3415	3415
query15	209	198	184	184
query16	942	418	389	389
query17	825	635	518	518
query18	2405	421	302	302
query19	189	166	137	137
query20	84	81	85	81
query21	215	122	104	104
query22	12938	13022	12917	12917
query23	13524	12902	12405	12405
query23_1	12623	12549	12558	12549
query24	7340	813	474	474
query24_1	473	474	471	471
query25	512	398	359	359
query26	1267	255	136	136
query27	2776	434	275	275
query28	4643	2031	2038	2031
query29	1549	575	448	448
query30	295	221	188	188
query31	1014	924	810	810
query32	155	98	97	97
query33	535	308	246	246
query34	918	851	499	499
query35	739	761	650	650
query36	856	832	733	733
query37	128	95	85	85
query38	1768	1716	1697	1697
query39	794	786	759	759
query39_1	731	726	734	726
query40	225	114	97	97
query41	71	71	102	71
query42	86	84	80	80
query43	343	358	309	309
query44	1320	702	709	702
query45	180	174	168	168
query46	834	976	564	564
query47	2950	2890	2773	2773
query48	296	308	223	223
query49	581	394	289	289
query50	677	277	208	208
query51	10419	10602	10469	10469
query52	77	79	69	69
query53	194	208	150	150
query54	230	190	184	184
query55	71	67	66	66
query56	238	213	211	211
query57	1662	1601	1569	1569
query58	289	264	278	264
query59	2187	2161	1973	1973
query60	284	244	229	229
query61	150	155	147	147
query62	391	330	293	293
query63	190	155	155	155
query64	2749	951	773	773
query65	3359	3277	3271	3271
query66	1834	422	299	299
query67	20039	20200	19801	19801
query68	3023	1016	624	624
query69	388	285	249	249
query70	866	787	775	775
query71	291	235	207	207
query72	2688	2522	2293	2293
query73	515	531	292	292
query74	4635	4492	4349	4349
query75	2356	2290	1897	1897
query76	2325	1009	596	596
query77	339	354	269	269
query78	9782	9684	9158	9158
query79	882	858	534	534
query80	474	406	331	331
query81	516	324	273	273
query82	199	134	104	104
query83	196	192	181	181
query84	291	131	107	107
query85	769	417	369	369
query86	269	237	233	233
query87	1983	1972	1784	1784
query88	3730	2781	2761	2761
query89	316	283	250	250
query90	2106	169	170	169
query91	161	153	128	128
query92	99	91	90	90
query93	977	975	577	577
query94	445	304	284	284
query95	575	314	363	314
query96	689	499	239	239
query97	2428	2483	2346	2346
query98	166	158	150	150
query99	761	774	655	655
Total cold run time: 233644 ms
Total hot run time: 151062 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.65 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 9b8cc64cf043ae1b6d3f53eab10c77cfe0c722ef, data reload: false

query1	0.01	0.00	0.01
query2	0.10	0.05	0.06
query3	0.26	0.09	0.09
query4	1.61	0.10	0.10
query5	0.23	0.25	0.26
query6	1.18	0.69	0.68
query7	0.04	0.01	0.01
query8	0.05	0.04	0.04
query9	0.42	0.36	0.36
query10	0.61	0.60	0.59
query11	0.20	0.16	0.14
query12	0.18	0.14	0.15
query13	0.51	0.50	0.49
query14	1.04	1.02	1.04
query15	0.65	0.63	0.63
query16	0.37	0.36	0.37
query17	1.14	1.18	1.11
query18	0.20	0.19	0.19
query19	2.10	1.92	2.06
query20	0.02	0.01	0.01
query21	15.45	0.20	0.12
query22	4.76	0.06	0.06
query23	16.28	0.33	0.12
query24	2.98	0.55	0.51
query25	0.10	0.06	0.07
query26	1.38	0.21	0.14
query27	0.06	0.03	0.05
query28	3.97	0.82	0.39
query29	12.50	4.19	3.31
query30	0.29	0.14	0.11
query31	2.78	0.58	0.33
query32	3.23	0.60	0.51
query33	3.27	3.18	3.18
query34	15.49	4.19	3.46
query35	3.40	3.34	3.35
query36	0.56	0.49	0.45
query37	0.10	0.07	0.06
query38	0.06	0.04	0.04
query39	0.03	0.03	0.04
query40	0.19	0.15	0.14
query41	0.09	0.03	0.02
query42	0.04	0.03	0.03
query43	0.05	0.04	0.03
Total cold run time: 97.98 s
Total hot run time: 24.65 s

Issue Number: N/A

Related PR: apache#68854, apache#69338

Problem Summary:
The Iceberg data, position-delete, and Hive Parquet writers created their output file before validating the requested Parquet compression codec. For an unsupported codec, the Iceberg data writer then attempted to delete an open file, while the other writers returned after creating the file. This made error cleanup depend on filesystem behavior and could leave an orphan file. Resolve the codec before calling create_file so invalid codecs fail without creating any output file, and add a regression test for the failure path.

None

- Test
    - [x] Unit Test: added UnsupportedParquetCodecDoesNotCreateFile; not executed locally per request, CI will run it.
    - [ ] Regression test
    - [ ] Manual test
    - [ ] No need to test or manual test. Explain why
- Behavior changed:
    - [x] No.
- Does this need documentation?
    - [x] No.
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69376

Problem Summary:
The negative codec test only checked that no file remained after open. The previous create-then-delete implementation could also satisfy that assertion. Instrument LocalFileSystem::create_file_impl through the existing sync point and assert it is never called, while retaining the null writer and final absence checks.

### Release note

None

### Check List (For Author)

- Test
    - [ ] Regression test
    - [x] Unit Test: strengthened UnsupportedParquetCodecDoesNotCreateFile; CI will run it.
    - [ ] Manual test
- Behavior changed:
    - [x] No.
- Does this need documentation?
    - [x] No.
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69376

Problem Summary: The unsupported-Parquet-codec regression test included
common/sync_point.h, which does not exist under the configured BE include roots.
This prevented the doris_be_test target from compiling. Include cpp/sync_point.h
and keep the include list ordered so the existing zero-file-creation assertions
can compile against the shared SyncPoint API.

### Release note

None

### Check List (For Author)

- Test: clang-format 16 for the four PR files, BE build hygiene, git diff --check,
  and a compiler probe that rejects the old include and accepts the corrected
  include and SyncPoint API usage all passed.
    - Unit Test: attempted run-be-ut.sh --run --filter=VIcebergPartitionWriterTest.*
      -j 2 with ASAN; stopped during the initial faiss submodule clone before
      compilation. No local BE unit-test runtime pass is claimed.
- Behavior changed: No; this commit corrects a test include only.
- Does this need documentation: No.
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69376

Problem Summary: The Iceberg data, position-delete and Hive partition writers
wrap a single filesystem create_file call in forwarding lambdas. Expand the
calls directly at each format branch to make the validation and file-creation
order explicit, while preserving every argument, writer option and error path.

### Release note

None

### Check List (For Author)

- Test: clang-format 16 via the repository formatter and git diff --check passed.
    - No need to rerun runtime tests: this mechanically expands seven existing
      forwarding calls without changing arguments or branch positions. Verified
      the remaining source tokens and existing negative test are unchanged.
- Behavior changed: No.
- Does this need documentation: No.
### Background

Iceberg writers carry separate physical and metadata directory forms in
IcebergOutputLocation. Their WriteInfo aggregate requires this location to be
initialized before a writer can be constructed.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#69376 (follow-up), apache#69392 (output-location interface change)
PR Type: Test

Problem Summary: The unsupported-Parquet-codec test still default-constructed
WriteInfo and assigned its removed write_path field. After merging current
master this prevents the BE unit-test target from compiling. Initialize the
physical and metadata directories explicitly while retaining the existing
zero-file-creation regression assertions.

### Details

- Failure: BE UT compilation reports a deleted WriteInfo default constructor
  and a nonexistent write_path member in the new codec-rejection test.
- Cause: the output-location refactor changed WriteInfo after the PR branched.
- Stability evidence: use the same aggregate pattern as current master tests,
  with both local directory forms set to '.', FILE_LOCAL, and no brokers.
  Keep the unique filename, SyncPoint callback and all assertions unchanged.

### Testing

- clang-format 16 on the four PR files, build hygiene, and git diff --check passed.
- git range-diff confirmed the four original patches are preserved by rebase.
- ASAN run-be-ut.sh --run
  --filter=VIcebergPartitionWriterTest.UnsupportedParquetCodecDoesNotCreateFile
  -j 2 was attempted locally, but stopped during dependency preparation before
  compilation. Required submodules were incomplete and the mandatory
  libgoogle_cloud_cpp_oauth2.a is absent. No local runtime test pass is claimed.

### Release note

None

### Check List (For Author)

- Test: static checks passed; local unit-test execution remains unverified.
- Behavior changed: No.
- Does this need documentation: No.
- Pre-submission status: Partially verified: BE UT compilation and execution
  need verification on the prepared CI environment.

### AI Disclosure

- Tool: Codex.
- Generated: test initialization and this commit message.
- Human review: Not performed in this automated session.
@hubgeter
hubgeter force-pushed the fix/iceberg-validate-parquet-compression-master branch from 9b8cc64 to 8df4f80 Compare November 5, 2026 03:22
@hubgeter

hubgeter commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor Author

run buildall

@hubgeter

hubgeter commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Local pipeline review — ✅ PASS

schema: doris-repo-review/v1
status: PASS
pr: apache/doris#69376
commit: 8df4f80b1f0824a91311db51ffc5a43d72c3b24c
base: 315c2e003317ef44e8a8fbb445a1ec8452d5647e
reviewed_at: 2026-11-05T03:41+00:00
reviewer: hubgeter
model: gpt-6.1-sol
effort: max
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: true

Notes for maintainers

  • Static review only; no new compilation, BE UT/regression, clang-tidy, filesystem/cluster or current-head CI pass was obtained. The local ASAN attempt stopped during dependency preparation before compilation/execution.
  • be/test/exec/sink/writer/iceberg/iceberg_partition_writer_test.cpp:214 explicitly initializes current-base IcebergOutputLocation; the real local zero-create gate and physical-versus-commit path interactions were rechecked at this head.
  • Reused 9 prior dispositions: D-03/D-07/D-09 by content checks, D-01/D-02/D-04/D-05/D-06/D-08 after changed-base/API refresh. Direct Hive/delete/remote codec-negative tests remain outside the added test scope.
  • Current-head CI is pending at the closing snapshot (BE UT 1093015, COMPILE 1093016, performance 1093017); successful buildall dispatch is not a compile/UT pass.

Reviewed locally with the doris-repo-review pipeline. Repository policy may accept this receipt for the matching commit; it is not a human Apache approval.

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17560	5221	5105	5105
q2	2131	306	256	256
q3	10257	1352	824	824
q4	4680	458	333	333
q5	7430	798	520	520
q6	181	185	153	153
q7	737	787	590	590
q8	9245	1262	1083	1083
q9	5439	4200	4153	4153
q10	6823	1287	1018	1018
q11	417	247	220	220
q12	630	392	276	276
q13	18111	3528	3036	3036
q14	283	263	252	252
q15	q16	731	699	654	654
q17	1163	744	563	563
q18	6963	6162	6120	6120
q19	1117	958	593	593
q20	386	326	235	235
q21	4483	2107	2297	2107
q22	355	297	257	257
Total cold run time: 99122 ms
Total hot run time: 28348 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5626	5558	5556	5556
q2	677	567	550	550
q3	5219	5401	4937	4937
q4	1975	2121	1532	1532
q5	5242	5165	5393	5165
q6	274	206	163	163
q7	2343	2115	1842	1842
q8	3042	2654	2671	2654
q9	8109	8029	7932	7932
q10	3711	3654	3599	3599
q11	595	427	406	406
q12	695	702	519	519
q13	3501	3933	3368	3368
q14	315	317	297	297
q15	q16	702	694	654	654
q17	8235	7309	6867	6867
q18	12978	12222	12953	12222
q19	896	760	773	760
q20	2151	2139	1925	1925
q21	5527	4578	4864	4578
q22	469	434	397	397
Total cold run time: 72282 ms
Total hot run time: 65923 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151615 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 8df4f80b1f0824a91311db51ffc5a43d72c3b24c, data reload: false

query5	4315	582	445	445
query6	428	205	187	187
query7	4801	434	224	224
query8	324	161	145	145
query9	8750	3161	3146	3146
query10	469	315	260	260
query11	6003	3851	3594	3594
query12	150	92	85	85
query13	1241	433	325	325
query14	6147	3952	3627	3627
query14_1	3473	3457	3433	3433
query15	207	195	176	176
query16	925	436	415	415
query17	848	654	540	540
query18	2406	429	317	317
query19	199	170	137	137
query20	90	101	83	83
query21	215	129	107	107
query22	13054	12962	12878	12878
query23	13408	12872	12388	12388
query23_1	12745	12824	12608	12608
query24	7325	815	479	479
query24_1	478	481	482	481
query25	532	420	368	368
query26	1273	260	139	139
query27	2750	444	273	273
query28	4568	2010	2027	2010
query29	1558	604	476	476
query30	306	227	192	192
query31	1011	918	832	832
query32	148	94	95	94
query33	533	303	247	247
query34	885	852	501	501
query35	746	741	648	648
query36	824	839	742	742
query37	127	96	90	90
query38	1761	1729	1687	1687
query39	786	799	742	742
query39_1	735	738	742	738
query40	223	117	100	100
query41	71	68	68	68
query42	89	83	88	83
query43	348	370	315	315
query44	1298	711	690	690
query45	184	181	164	164
query46	852	971	565	565
query47	2885	3071	2835	2835
query48	311	309	264	264
query49	571	392	300	300
query50	671	273	210	210
query51	10406	10384	10577	10384
query52	78	80	71	71
query53	187	212	147	147
query54	233	204	170	170
query55	75	64	61	61
query56	241	225	199	199
query57	1628	1598	1553	1553
query58	282	266	253	253
query59	2158	2159	1973	1973
query60	298	246	233	233
query61	150	144	145	144
query62	396	328	279	279
query63	190	157	154	154
query64	2565	962	750	750
query65	3329	3286	3274	3274
query66	1758	425	297	297
query67	20085	20293	20239	20239
query68	3011	1002	626	626
query69	374	291	235	235
query70	884	767	803	767
query71	299	236	211	211
query72	2628	2325	2252	2252
query73	516	522	294	294
query74	4611	4503	4365	4365
query75	2321	2281	1900	1900
query76	2326	996	571	571
query77	351	378	274	274
query78	9706	9770	9166	9166
query79	1057	923	552	552
query80	725	422	339	339
query81	563	315	285	285
query82	582	131	108	108
query83	269	196	176	176
query84	302	123	104	104
query85	827	425	376	376
query86	327	238	237	237
query87	2014	1960	1835	1835
query88	3744	2756	2778	2756
query89	336	287	245	245
query90	1844	163	157	157
query91	157	154	132	132
query92	99	88	89	88
query93	1015	1016	595	595
query94	456	306	281	281
query95	590	314	310	310
query96	690	487	236	236
query97	2435	2428	2320	2320
query98	164	151	148	148
query99	758	765	653	653
Total cold run time: 233859 ms
Total hot run time: 151615 ms

@hello-stephen

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

query1	0.01	0.01	0.00
query2	0.09	0.05	0.05
query3	0.26	0.09	0.08
query4	1.61	0.10	0.09
query5	0.23	0.24	0.24
query6	1.16	0.68	0.68
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.40	0.34	0.36
query10	0.61	0.58	0.60
query11	0.20	0.15	0.15
query12	0.19	0.15	0.14
query13	0.50	0.50	0.50
query14	1.02	1.00	1.01
query15	0.62	0.62	0.60
query16	0.36	0.35	0.32
query17	1.12	1.15	1.13
query18	0.19	0.18	0.17
query19	2.10	1.99	2.02
query20	0.02	0.02	0.01
query21	15.49	0.20	0.14
query22	4.89	0.05	0.05
query23	16.27	0.34	0.13
query24	2.80	0.51	0.44
query25	0.11	0.06	0.05
query26	1.63	0.22	0.15
query27	0.05	0.06	0.05
query28	4.22	0.70	0.36
query29	12.50	4.06	3.23
query30	0.29	0.13	0.10
query31	2.77	0.56	0.33
query32	3.22	0.60	0.49
query33	3.18	3.22	3.18
query34	15.38	3.99	3.42
query35	3.34	3.36	3.34
query36	0.58	0.44	0.41
query37	0.09	0.07	0.07
query38	0.04	0.04	0.04
query39	0.03	0.02	0.03
query40	0.17	0.14	0.15
query41	0.08	0.04	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 98 s
Total hot run time: 24.35 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.40% (32167/47727)
Line Coverage 52.27% (338755/648109)
Region Coverage 47.77% (275324/576372)
Branch Coverage 49.60% (129804/261706)

@Gabriel39
Gabriel39 merged commit 897bb25 into apache:master Nov 5, 2026
35 of 37 checks passed
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants