Skip to content

[fix](fe) Disable LOCAL TVF - #69591

Merged
924060929 merged 9 commits into
apache:masterfrom
OIiveirra:fix/local-tvf-default-off
Oct 27, 2026
Merged

924060929 merged 9 commits into
apache:masterfrom
OIiveirra:fix/local-tvf-default-off

Conversation

@OIiveirra

@OIiveirra OIiveirra commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: N/A

Related PR: #69526

Problem Summary:

LOCAL TVF can discover files on a backend during analysis. Reject LOCAL unconditionally before property parsing, backend selection and file discovery. FILE TVF with local storage delegates to the same entry point. Explicit LOCAL INSERT and analyzed EXPLAIN INSERT use the same rejection before sink property validation. No configuration option enables these paths, including for ADMIN/OPERATOR. The underlying storage implementation is retained.

Tests assert rejection before backend access and preserve remote FILE/sink behavior where supported. Existing LOCAL-dependent success suites explicitly return while the feature is unavailable. Export completion/error checks remain active; LOCAL export-readback comparisons are retired. Remote HDFS/S3 partition cases, the existing UUID S3 fixture branch, and ORC value checks through S3 TVF using the original uploaded fixtures remain active where present on this branch.

Release note

LOCAL TVF and FILE TVF with local storage are disabled for all users. There is no configuration option to re-enable them. Explicit LOCAL TVF INSERT is also disabled.

Check List (For Author)

  • Test
    • Unit Test — 13 targeted FE tests passed; zero failures, errors or skips.
    • Regression test — rejection coverage and affected-suite adaptations included; suite bodies have not been executed locally.
    • Manual test
    • No need to test

Validation: Checkstyle passed for all 6 changed Java files. Changed Groovy suites passed syntax parsing with Groovy 4.0.19. Whitespace checks passed. Runtime regression and formal local code review have not been performed.

  • Behavior changed:
    • No.
    • Yes. LOCAL admission always fails; no startup opt-in is available.
  • Does this need documentation?
    • No.
    • Yes. Release note included; operator documentation should reflect disabled LOCAL support.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69526

Problem Summary: Add startup-only enable_local_tvf=false. Reject LOCAL TVF and FILE-local at FE admission before discovery. Explicit opt-in retains the existing storage implementation and requires ADMIN/OPERATOR. Supported explicit LOCAL sinks use the same FE admission check.

### Release note

LOCAL TVF and FILE TVF with local storage require enable_local_tvf=true in fe.conf on every query-serving FE and an FE restart. This change provides default-disable mitigation; enabled storage-path sandbox hardening remains separate work.

### Check List (For Author)

- Test: Focused FE unit and regression cases added but not executed. Changed Java files pass Checkstyle; diff whitespace checks pass.
- Behavior changed: Yes. LOCAL access requires startup opt-in and ADMIN/OPERATOR admission.
- Does this need documentation: Yes. Configuration description and release note included; user-documentation follow-up remains pending.
@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?

@OIiveirra

Copy link
Copy Markdown
Collaborator 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 14 changed files at d376a62 using the supplied aggregate diff, two complete-review scopes and a separate admission-risk review. One P2 finding: the existing positive LOCAL sink binding test lacks the new startup opt-in and now throws before its assertions. No verified P0/P1 finding and no existing inline finding to carry forward. No additional user focus was supplied.

The main invocation paths checked were:

LOCAL / FILE-local SELECT, EXPLAIN, DESC
  -> LocalTableValuedFunction constructor -> checkAuth/checkAccess
  -> common properties -> backend discovery -> schema/scan
LOCAL INSERT / EXPLAIN INSERT
  -> UnboundTVFTableSink -> BindSink.checkAccess
  -> sink properties -> physical sink -> coordinator/write

Critical checkpoint conclusions:

  1. Goal and proof: the default-off gate covers explicit LOCAL and FILE-local query construction, plus explicit LOCAL sink analysis before destination handling. Added tests inspect disabled, enabled and denied cases. The existing positive sink fixture needs the inline fix; runtime behavior was not tested here.
  2. Scope and reuse: focused FE configuration/admission change; query and sink paths reuse one access routine. Deployment opt-ins and tests relate directly to the behavior change.
  3. Concurrency and locks: no new thread, lock, shared collection or lock order. Production reads a startup-loaded setting; no new I/O under locks.
  4. Lifecycle: startup loading precedes serving work. Memoized TVF objects, prepared statements and cache restrictions do not establish a same-process route around a startup-only setting. No new resource ownership or static initialization dependency.
  5. Configuration: enable_local_tvf defaults to false and ConfField makes it non-mutable. ConfigBase rejects runtime changes; DorisFE loads fe.conf then fe_custom.conf. Enabling it requires restarting every query-serving FE, as documented. Cloud deployment opt-ins use the existing startup configuration path.
  6. Compatibility: default disable is an intentional SQL behavior change described in the PR release note. Operators must opt in consistently across FEs. No incompatible function symbol, wire or storage-format change was found.
  7. Thrift/protobuf IDs: no gensrc changes; cross-branch field-ID checks are not applicable.
  8. Parallel paths: LOCAL, FILE-local, SELECT/EXPLAIN/DESC and LOCAL INSERT/EXPLAIN were traced. S3/HDFS paths remain separate. Local wildcard metadata delegation also constructs the gated LOCAL TVF. Exact-path parquet metadata is an unchanged separate API; this review does not claim a universal local-filesystem gate. Enabled storage-path containment is expressly separate PR scope.
  9. Conditions and planner stages: LOCAL admission follows name validation and precedes sink property checks. Existing rule registration/stage is preserved; no join, slot/ExprId, property-derivation or rewrite-loop change. Disabled and denied paths raise analysis errors.
  10. Test coverage: constructor tests check denial before backend/proxy interaction and enabled mocked discovery; authorization cases retain ADMIN/OPERATOR checks. Sink tests check denial and remote independence. M1 is the missed existing positive fixture. All seven supplied regression deployments opt in, so their new regression suite exercises the enabled backend-validation branch; default-off end-to-end deployment behavior and successful sink execution remain test-depth limits.
  11. Test results: no golden/.out result changes. The error-only regression uses test/sql/exception and requires no ordered result output. No builds or tests were run under this static-only contract.
  12. Observability: disabled errors name the setting and restart action; denied errors retain the existing privilege error. No new distributed operation or high-volume logging warrants added metrics here.
  13. Persistence and failover: no EditLog, replay, transaction-state or master-failover implementation changes. The startup option uses existing on-disk configuration loading.
  14. Writes and core invariants: sink rejection precedes destination side effects. Existing external-write atomicity and crash cleanup are unchanged. Committed-data visibility, visible versions, delete bitmaps and BE memory ownership are not modified by these FE admission checks.
  15. FE/BE variables: no added transport variable or changed serialization/send path; the gate is consumed in FE.
  16. Performance: constant-time configuration/privilege checks, with no added scan, copy, retry loop or large allocation; rejected requests avoid subsequent destination work.
  17. Other concerns and limits: the one verified finding is deduplicated across reviewers. Both initial admission risks were dismissed with concrete call-path evidence; no unresolved concrete candidate or unreviewed changed file remains. Convergence concerns this static review, not runtime validation. Builds, tests, service startup and enabled storage sandbox hardening were not performed.

Review converged after 2 round(s).

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591

Problem Summary: The existing positive LOCAL sink binding test reaches the new default-off gate before checking duplicate output positions. Save and enable the flag in fixture setup and restore it in teardown. Preserve the original LOCAL target, ROOT context and ExprId assertions.

### Release note

None

### Check List (For Author)

- Test: Changed-file Checkstyle and diff checks passed. Unit tests were not executed locally.
- Behavior changed: No production behavior change; existing test fixture opts in explicitly.
- Does this need documentation: No.
@OIiveirra

Copy link
Copy Markdown
Collaborator 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 15 changed files at caacf2c728408a2686721fec39c96dad54b59f61. No new actionable findings. Existing P2 thread 4372006062 is addressed: the duplicate-output LOCAL sink fixture now saves, enables and restores the flag, retaining its ROOT context and assertions. No independently confirmed existing P0/P1 finding remains.

  1. Goal and evidence: The default-false gate rejects LOCAL and FILE-local reads before property parsing/backend discovery, and local sinks during binding. Added constructor/binder tests and SELECT/EXPLAIN regression cases cover admission; execution results were not verified.
  2. Scope and reuse: The three production changes are focused; reads and sinks share checkAccess instead of duplicating admission logic.
  3. Concurrency and locking: No new runtime writer, thread or lock. Configuration loads before request serving; the flag is intentionally immutable at runtime. Test fixtures restore it, and FE test classes use separate JVM forks.
  4. Lifecycle and initialization: No new resource ownership, cleanup obligation or dependent static initializer. Reused TVFs still pass through existing authorization checks.
  5. Configuration: @ConfField defaults to non-mutable; ConfigBase loads the setting from startup configuration and rejects runtime changes. Enabling it requires configuring and restarting every FE, as described by the error and field documentation.
  6. Compatibility: Default disable and the local sink ADMIN/OPERATOR admission are intentional behavior changes. Command-level ADMIN/LOAD checks remain. No persisted format or existing function signature changes; operators must apply the policy consistently across FEs during upgrade.
  7. Thrift/protobuf: No definitions changed; cross-branch field-ID checks are inapplicable.
  8. Parallel paths: Reviewed direct LOCAL, FILE-local delegation, SELECT/EXPLAIN/DESCRIBE, later authorization and INSERT/EXPLAIN sink binding. Remote FILE delegates and S3/HDFS sink branches avoid the new gate. Local parquet-metadata wildcard expansion also inherits admission through its existing LOCAL helper.
  9. Conditions and errors: Disabled admission and denied privileges fail explicitly with Nereids analysis errors. Enabled requests retain downstream property/backend validation. Planner context identity is preserved; no silent continuation was introduced.
  10. Test coverage: Four FE test classes cover disabled/enabled reads, CSV/Lance, FILE-local versus remote dispatch, existing ADMIN/OPERATOR/revocation behavior, and local/remote sink binding. The existing valid sink fixture covers opt-in binding; seven pipeline configs explicitly enable LOCAL. No other concrete LOCAL FE fixture requiring opt-in was found.
  11. Expected results: No result files changed. The error-only regression uses test { sql; exception }; SHOW configuration returns the value at index 1, and the enabled backend-ID error occurs before file access.
  12. Observability: The rejection explains the setting and restart requirement. Existing privilege/backend diagnostics remain; no new distributed operation needs instrumentation.
  13. Transactions and persistence: No EditLog, replay, failover, visible-version or delete-bitmap changes; committed-data visibility invariants are unaffected.
  14. Writes and atomicity: Sink admission precedes physical sink execution and coordinator/file effects. Existing external-sink transaction and atomicity behavior is unchanged.
  15. FE-BE variables: No new transmitted field or alternate serialization path.
  16. Performance and memory: Constant-time analysis checks add no per-row work, loops, large allocations or memory-ownership changes.
  17. Other concerns and user focus: No additional user focus was supplied. Both initial risk items were dismissed with upstream/downstream evidence; no unresolved in-scope candidate remains.

Validation was static only: no builds, tests, service startup or runtime I/O were performed. In-repository cloud/vault deployment and cloud-p1 override wiring were inspected; ordinary hosted CI orchestration is absent from this checkout and was not verified. Unrelated local-file APIs and out-of-tree plugins were outside this review's scope.

Converged after 2 review rounds, including a final review of this unchanged payload by all three assigned reviewers. No new inline comments are proposed.

Review converged after 2 round(s).

@OIiveirra
OIiveirra marked this pull request as ready for review October 25, 2026 19:28
Copilot AI balanced review requested due to automatic review settings October 25, 2026 19:28

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	12012	5103	5054	5054
q2	1305	302	261	261
q3	3220	1371	813	813
q4	831	457	340	340
q5	1097	809	573	573
q6	187	189	160	160
q7	807	874	595	595
q8	976	881	956	881
q9	4285	4262	4243	4243
q10	1498	1267	1036	1036
q11	387	264	233	233
q12	361	391	276	276
q13	3370	3607	3001	3001
q14	275	267	247	247
q15	q16	717	724	685	685
q17	1165	749	589	589
q18	6844	6181	6144	6144
q19	659	950	592	592
q20	386	330	223	223
q21	3028	2324	2299	2299
q22	347	303	268	268
Total cold run time: 43757 ms
Total hot run time: 28513 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5619	5623	5617	5617
q2	676	559	533	533
q3	5217	5403	4900	4900
q4	1999	2174	1536	1536
q5	5314	5191	5179	5179
q6	236	184	141	141
q7	2116	1872	1675	1675
q8	2857	2506	2537	2506
q9	7564	7530	7512	7512
q10	3669	3605	3208	3208
q11	602	413	386	386
q12	666	658	516	516
q13	3218	3619	3068	3068
q14	305	300	276	276
q15	q16	665	703	618	618
q17	7792	7026	6904	6904
q18	12981	12232	12868	12232
q19	906	789	794	789
q20	2145	2177	1928	1928
q21	5578	4579	4703	4579
q22	517	495	435	435
Total cold run time: 70642 ms
Total hot run time: 64538 ms

@hello-stephen

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

query5	4298	589	452	452
query6	423	214	199	199
query7	4802	448	224	224
query8	309	159	148	148
query9	8770	3093	3149	3093
query10	468	312	259	259
query11	5931	3881	3570	3570
query12	141	92	85	85
query13	1243	431	323	323
query14	6120	3952	3947	3947
query14_1	3447	3445	3399	3399
query15	217	196	181	181
query16	926	453	433	433
query17	1058	626	497	497
query18	2413	414	300	300
query19	185	153	127	127
query20	83	86	83	83
query21	207	121	102	102
query22	12907	13058	12829	12829
query23	13489	12810	12407	12407
query23_1	12458	12427	12558	12427
query24	7362	829	458	458
query24_1	471	464	460	460
query25	510	397	332	332
query26	1256	256	138	138
query27	2758	440	265	265
query28	4621	1931	1937	1931
query29	1545	572	450	450
query30	302	224	190	190
query31	1022	940	827	827
query32	154	109	93	93
query33	545	315	251	251
query34	911	883	511	511
query35	752	779	673	673
query36	819	848	741	741
query37	128	100	86	86
query38	1813	1758	1680	1680
query39	811	806	764	764
query39_1	743	738	747	738
query40	216	123	104	104
query41	75	70	68	68
query42	91	95	86	86
query43	351	360	312	312
query44	1308	689	693	689
query45	186	181	172	172
query46	828	974	564	564
query47	3043	3044	2773	2773
query48	326	323	240	240
query49	587	407	292	292
query50	676	288	212	212
query51	10389	10216	10507	10216
query52	80	93	72	72
query53	191	208	153	153
query54	246	200	205	200
query55	74	70	67	67
query56	235	244	238	238
query57	1672	1624	1587	1587
query58	298	283	264	264
query59	2101	2232	1959	1959
query60	306	259	240	240
query61	170	166	172	166
query62	385	336	297	297
query63	188	156	165	156
query64	2888	910	732	732
query65	3483	3293	3274	3274
query66	1775	412	293	293
query67	19975	20011	20054	20011
query68	2945	999	606	606
query69	373	279	253	253
query70	819	758	793	758
query71	312	237	205	205
query72	2721	2606	2261	2261
query73	508	532	296	296
query74	4666	4526	4343	4343
query75	2349	2288	1935	1935
query76	2308	1003	593	593
query77	334	360	253	253
query78	9718	9753	9159	9159
query79	980	896	511	511
query80	694	432	333	333
query81	549	320	270	270
query82	567	131	104	104
query83	268	202	177	177
query84	259	120	101	101
query85	888	420	365	365
query86	343	244	227	227
query87	2004	1971	1806	1806
query88	3702	2668	2617	2617
query89	334	277	234	234
query90	1777	168	159	159
query91	160	154	123	123
query92	100	86	89	86
query93	1023	981	578	578
query94	485	321	244	244
query95	565	327	376	327
query96	656	486	230	230
query97	2459	2429	2309	2309
query98	162	149	147	147
query99	787	759	661	661
Total cold run time: 234013 ms
Total hot run time: 150725 ms

@hello-stephen

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

query1	0.00	0.00	0.01
query2	0.09	0.05	0.05
query3	0.26	0.10	0.09
query4	1.61	0.10	0.10
query5	0.23	0.25	0.24
query6	1.17	0.68	0.66
query7	0.04	0.00	0.00
query8	0.05	0.04	0.04
query9	0.41	0.34	0.35
query10	0.57	0.56	0.59
query11	0.22	0.15	0.15
query12	0.18	0.15	0.14
query13	0.51	0.51	0.50
query14	1.03	1.02	1.00
query15	0.63	0.61	0.60
query16	0.34	0.35	0.34
query17	1.14	1.09	1.15
query18	0.21	0.20	0.20
query19	2.05	1.95	1.92
query20	0.01	0.02	0.01
query21	15.47	0.22	0.14
query22	4.75	0.05	0.05
query23	16.28	0.32	0.12
query24	3.22	0.48	0.55
query25	0.13	0.05	0.06
query26	1.37	0.22	0.15
query27	0.06	0.03	0.03
query28	3.74	0.74	0.38
query29	12.48	4.12	3.26
query30	0.28	0.12	0.12
query31	2.77	0.56	0.33
query32	3.25	0.60	0.50
query33	3.25	3.26	3.30
query34	15.40	4.00	3.43
query35	3.38	3.35	3.35
query36	0.54	0.45	0.43
query37	0.10	0.06	0.07
query38	0.05	0.03	0.04
query39	0.04	0.03	0.02
query40	0.18	0.15	0.14
query41	0.09	0.04	0.03
query42	0.03	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 97.65 s
Total hot run time: 24.45 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 100.00% (6/6) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 66.67% (4/6) 🎉
Increment coverage report
Complete coverage report

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69526

Problem Summary: Disable LOCAL and FILE-local admission without an opt-in setting, before file discovery. Explicit LOCAL INSERT and analyzed EXPLAIN INSERT use the same rejection before sink property validation. Retain storage implementation and adapt only directly affected fixtures and suites.

### Release note

LOCAL TVF and FILE TVF local storage are unavailable for all users, with no enable setting. LOCAL TVF INSERT is also unavailable.

### Check List (For Author)

- Test: 13 targeted FE unit tests passed without failures/skips; changed Java Checkstyle, Groovy syntax and whitespace checks passed. Regression suite bodies not executed.
- Behavior changed: Yes. Unconditional LOCAL rejection.
- Does this need documentation: Yes. Release note included; operator documentation follow-up.
OIiveirra added a commit to OIiveirra/doris that referenced this pull request Oct 26, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591, apache#69527

Problem Summary: Disable LOCAL and FILE-local admission without an opt-in setting, before file discovery. Explicit LOCAL INSERT and analyzed EXPLAIN INSERT use the same rejection before sink property validation. Retain storage implementation and adapt only directly affected fixtures and suites.

### Release note

LOCAL TVF and FILE TVF local storage are unavailable for all users, with no enable setting. LOCAL TVF INSERT is also unavailable.

### Check List (For Author)

- Test: 8 targeted FE unit tests passed without failures/skips; changed Java Checkstyle, Groovy syntax and whitespace checks passed. Regression suite bodies not executed.
- Behavior changed: Yes. Unconditional LOCAL rejection.
- Does this need documentation: Yes. Release note included; operator documentation follow-up.
OIiveirra added a commit to OIiveirra/doris that referenced this pull request Oct 26, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591, apache#69528

Problem Summary: Disable LOCAL and FILE-local admission without an opt-in setting, before file discovery. Explicit LOCAL INSERT and analyzed EXPLAIN INSERT use the same rejection before sink property validation. Retain storage implementation and adapt only directly affected fixtures and suites.

### Release note

LOCAL TVF and FILE TVF local storage are unavailable for all users, with no enable setting. LOCAL TVF INSERT is also unavailable.

### Check List (For Author)

- Test: 8 targeted FE unit tests passed without failures/skips; changed Java Checkstyle, Groovy syntax and whitespace checks passed. Regression suite bodies not executed.
- Behavior changed: Yes. Unconditional LOCAL rejection.
- Does this need documentation: Yes. Release note included; operator documentation follow-up.
@OIiveirra OIiveirra changed the title [fix](fe) Disable LOCAL TVF by default [fix](fe) Disable LOCAL TVF Oct 26, 2026
@OIiveirra

Copy link
Copy Markdown
Collaborator 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 head 2494b0c91859324f13bf9845dbb64eb299071377 found one new P2 regression: existing Parquet metadata local-glob tests still expect the now-disabled constructor to succeed. The unconditional LOCAL admission checks match the stated behavior; the affected metadata tests need adaptation. No P0/P1 findings.

All 41 changed paths and relevant upstream/downstream callers were covered. Critical checkpoint conclusions:

Checkpoint Conclusion
Goal and test proof LOCAL and FILE-local construction, DESC, explicit LOCAL INSERT and analyzed EXPLAIN reach unconditional rejection before local discovery/sink execution. Negative tests cover malformed properties, formats, privileges, delegation and sink analysis. Parsed-only EXPLAIN performs no analysis or file access. MAIN-1 identifies the missed existing regression dependency.
Scope and clarity Two small production changes share one rejection method; other changes adapt dependent tests.
Concurrency and locking No threads, shared mutable state or locks added; rejection precedes backend futures/coordinators. Existing lock order is unchanged.
Lifecycle and initialization No new owned resources or static initialization dependencies; constructor rejection precedes resource acquisition, and test mocks are closed.
Configuration No configuration added; rejection has no ADMIN/OPERATOR or startup opt-in.
Compatibility and upgrades Intentional SQL feature retirement is described in the release note. No storage-format or wire compatibility changes; enforcement remains local to the FE version handling the statement.
Thrift/protobuf No definitions, field IDs or enum values changed.
Parallel paths LOCAL factory/Nereids, FILE-local, DESC and sink/EXPLAIN paths traced. Remote delegates remain available. The Parquet metadata glob helper is the overlooked dependency reported inline.
Conditional checks Sink name is normalized before the LOCAL-only check; S3/HDFS retain their validation. Constructor rejection is unconditional.
Test coverage New rejection tests and retained authorization/duplicate-output checks inspected. Remote UUID, partition and export-view branches remain reachable. MAIN-1 is the outstanding test regression.
Expected results No output files changed. ORC fixture schemas/counts match destinations and existing expected counts (2/25000); value assertions and golden-tag lookup remain valid.
Observability The analysis error identifies disabled LOCAL admission; existing error handling remains sufficient. No new distributed operation needs metrics.
Transactions/persistence/failover No production transaction, EditLog, replay or version-visibility changes.
Data writes/atomicity/crashes LOCAL sinks reject before file deletion/output execution. Remote writes are unchanged; migrated test loads use existing failure/filter/publication checks and sync.
FE/BE propagation No new transmitted variables or sending paths.
Performance Constant-time admission checks; no new scans, backend calls or execution-loop work.
Other/module invariants No additional verified issue. FE locking/exception and Nereids registration/stage/property checkpoints checked; joins, slots, visible versions, delete bitmaps and BE memory ownership are unchanged.

Existing thread 4372006062 is resolved at this head: the duplicate-output fixture now uses S3 and retains its ExprId assertions. It is not reposted; no existing P0/P1 finding remains to carry forward. The metadata-glob finding concerns a distinct caller and suite.

User focus: no additional focus was supplied.

Validation limits: static review only; no builds, tests or cluster operations were run. LOCAL-dependent suites and export LOCAL readback comparisons are intentionally retired, reducing that coverage. Runtime regression behavior and external service availability remain untested. No unresolved candidate or uncovered assigned static scope remains.

Review converged after 2 round(s).

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591

Problem Summary: Parquet metadata local wildcard expansion delegates to the disabled LOCAL TVF constructor. Expect the admission error for both the former successful glob and the no-match glob. Preserve all literal-path and remote metadata checks.

### Release note

None

### Check List (For Author)

- Test: Groovy 4.0.19 syntax parsing and whitespace checks passed; all other suite blocks verified byte-for-byte unchanged. Runtime regression not executed locally. No Java source changed.
- Behavior changed: No. Test expectations follow the existing unconditional rejection.
- Does this need documentation: No.
OIiveirra added a commit to OIiveirra/doris that referenced this pull request Oct 26, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591

Problem Summary: Parquet metadata local wildcard expansion delegates to the disabled LOCAL TVF constructor. Expect the admission error for both the former successful glob and the no-match glob. Preserve all literal-path and remote metadata checks.

### Release note

None

### Check List (For Author)

- Test: Groovy 4.0.19 syntax parsing and whitespace checks passed; all other suite blocks verified byte-for-byte unchanged. Runtime regression not executed locally. No Java source changed.
- Behavior changed: No. Test expectations follow the existing unconditional rejection.
- Does this need documentation: No.

@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 44 changed files at head 0977874 using static inspection. One new P2 finding: the two ORC Stream Load migrations target ARRAY/MAP columns, which the mandatory load scanner rejects before the preserved assertions. The LOCAL admission changes otherwise implement the intended unconditional shutdown.

The three existing P2 threads are addressed at this head: the duplicate-output sink fixture uses S3; matching/no-match local metadata globs expect the disabled error; the ordinary-user account cases expect that error and UUID export uses its existing S3 path. These are not reposted. No existing P0/P1 inline finding applies, so existing_blocking_comment_ids is empty. No additional user focus was supplied.

  • Goal and proof: LOCAL and FILE-local construction reject before local property parsing/discovery, and explicit LOCAL sink binding rejects before sink validation. Constructor, privilege, format and INSERT/analyzed-EXPLAIN negative tests cover these entry points. The intended preservation of the two complex-data regression suites fails as described inline.
  • Scope and clarity: the production change is a shared rejection called at two admission sites; accompanying test changes follow the stated feature removal. The unsupported load replacement needs correction.
  • Concurrency and locking: the rejection is stateless and adds no threads, mutable shared state, locks, lock-order changes or work inside locks. Retained test uploads use unique object keys/labels.
  • Lifecycle: rejection precedes local backend RPCs and sink coordinator/query registration. Existing planning cleanup and test mock teardown remain; no new ownership or initialization dependency was found.
  • Configuration: no configuration is added. Shutdown is unconditional for privileged users too; dynamic configuration propagation does not apply.
  • Compatibility: disabling previously supported SQL is intentional and described in the PR release note. No storage format or wire contract is changed; during a rolling FE upgrade admission follows the analyzing FE's version.
  • Thrift/protobuf: no definitions, field IDs or enum values are changed.
  • Parallel paths: LOCAL, FILE-local, SELECT, aliases, DESCRIBE, analyzed EXPLAIN and explicit LOCAL INSERT routes were traced. Parse-only EXPLAIN prints syntax without file access. Remote FILE/sink delegates and exact-path/remote metadata routes remain separate. Both complex ORC migrations share the reported defect.
  • Conditional checks: the sink dispatch uses the already normalized function name and the shared LOCAL name. Negative expectations match the unconditional error; suite skips occur at closure entry.
  • Test coverage: all six Java and 38 Groovy changes were reviewed. The PR intentionally skips 22 LOCAL-dependent suites and retires local export readback comparisons; these are coverage losses. Retained UUID S3, HDFS/S3 partition, metadata and export lifecycle cases were inspected. The two migrated ORC suites cannot provide their intended coverage until the finding is fixed.
  • Test results: no .out files change. Existing 2-row/25,000-row count goldens, repeated tag order and skipped golden-block handling are compatible in isolation; the load rejection occurs before those comparisons. No builds, tests or runtime suites were executed in this review.
  • Observability: the immediate admission error identifies disabled LOCAL, and skipped suites log their reason. Existing exception/status handling exposes failures; no new runtime metrics are needed for this rejection.
  • Transactions and persistence: no journal, replay, visible-version or transaction implementation changes. Existing export completion/cancellation/retry checks remain; the LOCAL sink fails before execution.
  • Writes, atomicity and crashes: remote write implementations are unchanged. The reported migration error is a failed load before value assertions, with no data-loss claim; no new crash-cleanup or atomicity defect was found in changed code.
  • FE/BE variables: none are added. Existing Stream Load source/destination tuple serialization and scanner selection were traced to establish the finding.
  • Performance: constant-time LOCAL rejection avoids property/discovery work; no new repeated scans, large allocations or hot-path loops are introduced.
  • Other issues and core invariants: the final recheck covered exception types, static mocks, stale callers, removed variables, fixture schemas and scanner alternatives. No additional defect was substantiated. Committed visibility, partition versions, delete bitmaps and BE memory accounting have no changed implementation in this PR; admission failures propagate explicitly.

Review converged in round 2 after independent checks of this exact finding set and summary. Coverage is static only; runtime behavior was inferred from the traced code, and intentionally retired LOCAL success coverage is not restored by this review.

Review converged after 2 round(s).

@shuke987

Copy link
Copy Markdown
Contributor

run buildall

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591

Problem Summary: The ARRAY and MAP regression fixtures were migrated from LOCAL TVF to ORC Stream Load, but the load scanner rejects complex destination columns. Upload the original fixtures to unique temporary S3 objects and use INSERT SELECT from S3 TVF, cleaning up each object after the insert. Preserve table definitions, synchronization and all existing count/value checks.

### Release note

None

### Check List (For Author)

- Test: Groovy 4.0.19 syntax parsing, ORC fixture schema/row-count/content inspection and git diff --check passed. Runtime regression was not executed locally; matching FE/BE artifacts are unavailable in these task worktrees.
- Behavior changed: No (test data-loading adaptation only)
- Does this need documentation: No
@OIiveirra

Copy link
Copy Markdown
Collaborator 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 head 215f8dd covered all 44 changed files (6 Java and 38 Groovy), related invocation paths, and the supplied existing threads. One new P2 finding concerns Azure compatibility of the two migrated ORC fixture uploads. No new P0/P1 issue was verified.

Critical checkpoint conclusions:

  1. Goal and evidence: LOCAL source construction and explicit LOCAL sink analysis reject unconditionally; FILE(local), SELECT/EXPLAIN, and analyzed INSERT/EXPLAIN reach these gates. Source/sink unit tests and account regression assertions cover early rejection and retained remote validation; inspected only.
  2. Scope: production edits are small and centralized. Regression adaptations follow the intentional LOCAL disablement; the fixture-provider regression is reported inline.
  3. Concurrency and locks: no production shared state, threads, locks, or lock ordering changes. Test static mocks are scoped and closed.
  4. Lifecycle and initialization: source rejection precedes backend discovery; sink rejection precedes writer/coordinator setup. ORC fixture objects are deleted in finally after synchronous INSERT completion. No new static initialization dependency.
  5. Configuration: no admission flag or other production configuration is added. Existing Azure regression configuration exposes the reported upload-client mismatch.
  6. Compatibility: LOCAL SQL behavior intentionally changes as documented in the release note; wire layouts, persisted formats, and function signatures are unchanged.
  7. Thrift/protobuf: no definitions or IDs changed.
  8. Parallel paths: checked direct LOCAL, FILE delegation, registration/DESC construction, sink INSERT/analyzed EXPLAIN, and metadata wildcard expansion. Remote source/sink branches remain outside the new rejection. Exact-path metadata queries bypass wildcard expansion.
  9. Conditional checks: the added sink condition uses the normalized function name, and the shared rejection documents its early-failure purpose. Existing command-level INSERT privileges can still reject before planning.
  10. Test coverage: new negative unit and account tests cover malformed properties, formats, privilege values, no backend interaction, and remote validation. Existing LOCAL-only success suites are intentionally skipped; local export readback coverage is retired while export completion/cancellation and surviving remote cases remain. Runtime/build validation was not performed under this static-only contract.
  11. Results: no .out files changed. Framework tag lookup tolerates retired tags; fixture schemas and row counts match the explicit ORC projections and unchanged count results (2 array rows; 25,000 map rows). Ordered array-value checks remain.
  12. Observability: disabled statements return an explicit analysis error; suite skip logging and export-state/error checks remain. No added distributed operation needs new metrics.
  13. Transactions and persistence: no EditLog, replay, visible-version, delete-bitmap, or transaction lifecycle change.
  14. Writes and atomicity: LOCAL fails before file writing/deletion. Existing remote execution is unchanged; fixture upload compatibility is the reported test failure, with no verified data-visibility or atomicity regression.
  15. FE/BE variables: no new transmitted variables or serialization paths.
  16. Performance and memory: the gate is constant-time and avoids LOCAL discovery; no new hot-path scans, large allocations, or BE memory-ownership changes.
  17. Additional review: checked wrapper exception propagation, indirect constructor users, retained test branches, and duplicate-output ExprIds. No additional verified issue or unresolved static code question remains.

Existing findings: all four supplied P2 threads are addressed at this head: the sink binding fixture uses S3; metadata wildcard cases expect disablement; account errors and UUID export use the updated paths; complex ORC fixtures load through S3 TVF rather than unsupported Stream Load. The new Azure-provider finding is distinct from that earlier ORC-format issue. No existing P0/P1 finding needs carrying forward, and no duplicate inline finding is posted.

User focus: none supplied beyond a complete review. Coverage limit: conclusions are based on static source/diff and fixture inspection; no builds, unit tests, regression suites, or live provider operations were executed.

Review converged after 2 round(s).

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591

Problem Summary: The migrated ARRAY and MAP fixtures always used an AWS client even when the regression provider was Azure. Select the existing Azure Blob SDK for Azure upload/cleanup and pass the selected provider and endpoint to S3 TVF. Resolve the framework Azure endpoint placeholder while preserving explicit endpoints. Keep S3 uploads, original fixtures and all existing assertions.

### Release note

None

### Check List (For Author)

- Test: Eight scripts compiled with the existing SDK dependencies; 12 loopback SDK upload/cleanup scenarios passed, including mixed-case Azure, default/explicit endpoints and injected SQL failure. SQL was intercepted in this probe; no Doris runtime regression or live cloud validation was performed. git diff --check passed.
- Behavior changed: No (test data-loading adaptation only)
- Does this need documentation: No
@OIiveirra

Copy link
Copy Markdown
Collaborator 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 all 44 changed files at 30ce3e7 found one new P2 regression: the three UUID fixture suites that now always upload with the AWS client lose their previously working Azure-plus-local-fixture configuration. The inline comment identifies that precise configuration; the already-broken Azure fallback without local overrides is not claimed as new.

Critical checkpoint conclusions:

  • Goal and evidence: LOCAL construction and explicit LOCAL sink binding now reject admission unconditionally. Added FE tests cover rejection before file access/property validation, privileged users, FILE local dispatch, INSERT and analyzed EXPLAIN, and unchanged remote validation. Tests were inspected, not executed.
  • Scope and clarity: production changes are two small edits using one stateless rejection helper. Most changed files adapt tests to deliberate LOCAL removal.
  • Concurrency and locks: no production threads, shared mutable state, locks or lock-order changes. Fixture object names are unique.
  • Lifecycle and initialization: rejection precedes backend/RPC resource acquisition. Test static mocks close after each case; ORC fixture cleanup follows synchronous INSERT in finally. No new static-initialization dependency.
  • Configuration: no new configuration or opt-in; the intended rejection is unconditional.
  • Compatibility: LOCAL reads/writes are intentionally disabled, including ADMIN/OPERATOR. No storage-format or wire-symbol change requires rolling-upgrade adaptation; the PR release note describes the behavior change.
  • Thrift/protobuf: no definitions changed.
  • Parallel paths: direct LOCAL, FILE with local storage, and explicit LOCAL sinks reach rejection. Remote FILE delegates and S3/HDFS sinks remain separate. Local metadata wildcard expansion also rejects through LOCAL; explicit-path metadata remains separate.
  • Conditional checks: the sink name is normalized before the local-only check; placement is documented and precedes sink-property validation. Error propagation preserves the disabled message.
  • Test coverage: all four Java test files and all 38 Groovy changes were reviewed. Local-only suites intentionally return; mixed suites retain remote checks. The new P2 concerns three different UUID loaders from the previously fixed ORC loaders.
  • Expected results: no golden files changed. ORC footer field names/counts match the new projections and existing results (2 array rows, 25000 map rows); replacement COUNT queries preserve tags/values. The framework skips retired golden blocks by tag.
  • Observability: existing analysis/query errors expose rejection; no new distributed operation needs additional production metrics or logging.
  • Transactions and persistence: no journal, replay, failover or persisted metadata logic changed.
  • Writes and atomicity: LOCAL sinks fail before physical sink/coordinator execution. Production remote writes and atomicity are unchanged; fixture upload failures remain visible to the test framework.
  • FE/BE variables: no transmitted field or session variable added.
  • Performance and memory: the new sink check is constant-time; rejected LOCAL avoids property parsing and backend discovery. No new large allocation or repeated scan; visibility, delete-bitmap and memory-tracking mechanisms are unchanged.
  • Additional issues: the final missed-issue checks covered surviving callers, exception families, analyzed EXPLAIN, provider routing, cleanup, fixture schemas and output alignment; no second verified new defect was found.

All five existing P2 thread subjects were revalidated as addressed at this head: the sink binding fixture, local metadata glob expectations, account/UUID-export adaptation, complex-ORC loading, and Azure upload for those two ORC loaders. They are not reposted. No existing P0/P1 finding was present, so existing_blocking_comment_ids is empty. No additional user focus was supplied.

Coverage limits: static analysis only, as required. No builds, tests, cluster execution, SDK loopback probes or live-cloud validation were performed in this invocation. Local-only success/readback coverage is deliberately retired by this PR. There are no unresolved code candidates or assigned coverage gaps after the final reviewer pass.

Review converged after 2 round(s).

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591

Problem Summary: Removing the disabled LOCAL fallback makes three UUID fixture suites use an AWS upload even for Azure. Use the already-declared Azure Blob SDK for Azure uploads and pass the matching provider/endpoint to TVF. Preserve explicit endpoints, the existing S3 upload path, original fixtures and all schema/value/profile assertions. This change is limited to these three existing suites.

### Release note

None

### Check List (For Author)

- Test: Six scripts compiled with existing SDK dependencies. Twelve loopback SDK scenarios verified the three exact UUID setup/upload blocks, Azure default/explicit endpoints, S3 uploads, byte-identical fixtures, matching TVF parameters and fixture availability for later queries. No SQL execution, Doris runtime regression or live cloud validation was performed. git diff --check passed.
- Behavior changed: No (test data-loading adaptation only)
- Does this need documentation: No
@OIiveirra

Copy link
Copy Markdown
Collaborator 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 all 44 changed files at ef4eca4 found one new P2 regression: UUID EXPORT/readback still lacks Azure provider/endpoint adaptation after removal of its LOCAL override. This is a separate SQL export path from the fixture uploaders in the existing Azure discussions.

Critical checkpoint conclusions:

  1. Goal and evidence: unconditional LOCAL reads, FILE-local reads and LOCAL sink binding reject before local discovery or sink setup. New FE and account tests cover rejection, privileged users, EXPLAIN and preserved remote validation; inspected statically only.
  2. Scope and reuse: the two production edits share one rejection method. Regression changes retire LOCAL-dependent success paths and retain independent remote cases.
  3. Concurrency and locks: no new threads, shared mutable state, locks or lock ordering; rejection precedes local RPC and coordinator work.
  4. Lifecycle and static initialization: no new static initialization dependency. Static mocks close after each test; array/map fixture objects are deleted in finally after synchronous INSERT completes.
  5. Configuration: no new setting or enable flag; dynamic propagation is inapplicable.
  6. Compatibility: disabling LOCAL SQL is intentional and release-noted. No incompatible serialized format or FE-BE protocol change was found; remote delegates remain available.
  7. Thrift/protobuf: no definitions changed; cross-branch field-id checks are inapplicable.
  8. Parallel paths: inspected direct LOCAL, FILE delegation, TVF factories, SELECT/INSERT and analyzed EXPLAIN, metadata wildcard expansion, and unchanged callers. The remaining provider failure is reported inline.
  9. Conditions and errors: normalized sink-name selection gates LOCAL before sink property validation. Constructor rejection reaches existing analysis-error reporting. Earlier INSERT privilege rejection remains unchanged.
  10. Test coverage: reviewed all four changed FE test classes and all 38 regression scripts. Negative tests use exception assertions. Retained remote schema/value/profile assertions remain; LOCAL content, cache and roundtrip coverage is deliberately retired.
  11. Expected results: no golden files changed. The framework skips unused retired tags; replacement array/map count checks match existing values of 2 and 25000, and original fixtures/schemas are preserved.
  12. Observability: the disabled message and skip logs explain the changed behavior; no new distributed operation requires metrics or additional tracing.
  13. Transactions and persistence: no EditLog, replay, visible-version, delete-bitmap or failover behavior changes.
  14. Writes and atomicity: rejected LOCAL sinks stop during analysis before deletion, query registration or BE execution. Remote fixture lifetimes cover synchronous loads; no additional cleanup or atomicity defect was substantiated.
  15. FE-BE variables: no new transmitted variables or fields.
  16. Performance and memory: production rejection is constant-time and avoids discovery work; no new BE allocation or unbounded scan/retry pattern was introduced. Fixture uploads are a fixed finite set with unique keys.
  17. Other issues: further sweeps found no additional actionable issue or unresolved static candidate. No additional user focus was supplied.

Existing threads were independently revalidated and not reposted: the sink fixture now uses S3; metadata/account cases expect rejection; ORC complex-column fixtures use remote TVF; the five fixture uploaders select Azure where needed. The UUID export provider instance reported here remains distinct. No existing P0/P1 inline finding requires retention.

Limits: review was static only; no builds, tests, SQL, cluster or live-cloud validation were performed. Deliberately retired LOCAL success/readback coverage is not replaced by runtime validation in this review.

Review converged after 2 round(s).

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69591

Problem Summary: UUID export/readback forwards the Azure endpoint placeholder without its provider after the LOCAL fallback is removed. Resolve the Azure endpoint and include provider in the existing shared properties used by both EXPORT and the readback TVF. Keep all fixture data, export state/error handling and ordered result assertions unchanged.

### Release note

None

### Check List (For Author)

- Test: Both Groovy scripts compiled. Eight intercepted-SQL scenarios verified shared export/readback properties for Azure default/explicit endpoints and OSS, retained export-state handling and ordered golden-check invocations. No Doris SQL execution or live storage validation was performed. git diff --check passed.
- Behavior changed: No (test data-loading adaptation only)
- Does this need documentation: No
@OIiveirra

Copy link
Copy Markdown
Collaborator Author

run buildall

@OIiveirra

Copy link
Copy Markdown
Collaborator 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 exact head d197857e663ea16cac712e6b2081b2650d8d5c6b. No new actionable findings. All seven existing P2 reports were independently revalidated and are addressed in the current code; there are no existing P0/P1 findings to carry forward.

Coverage includes all 44 changed files (six Java and 38 Groovy), related analyzer/storage/export paths, regression framework behavior and existing golden results. Two complete-scope reviewers and a separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS; all initial risk questions have evidence-backed dispositions.

Critical checkpoint conclusions:

  1. Goal and evidence: LOCAL construction rejects before authorization, LOCAL property parsing, backend selection and file discovery. FILE-local delegates to that constructor. Explicit LOCAL INSERT and analyzed EXPLAIN reach the sink-binding rejection before sink-property validation or execution. Inspected FE tests and account regressions encode these negatives and remote controls.
  2. Scope and reuse: the production change is two small gates sharing one rejection method. The broader test changes adapt consumers to the intentional withdrawal of LOCAL support.
  3. Concurrency and locking: no new threads, shared mutable production state, locks, lock ordering or blocking operations are introduced.
  4. Lifecycle and initialization: no production ownership or static-initialization change. Unit-test static mocks close in teardown; migrated ORC fixtures use unique object keys and matching-provider cleanup in finally. UUID remote-fixture lifetime follows the existing remote branch.
  5. Configuration: no enable flag or dynamic opt-in is added; rejection is unconditional.
  6. Compatibility: this intentionally changes SQL behavior for LOCAL, FILE-local and explicit LOCAL sinks for all users, as stated in the release note. Storage formats and serialized state are unchanged.
  7. Thrift/protobuf: no definitions change; field-ID and rolling-protocol checks are inapplicable.
  8. Parallel paths and stage order: direct LOCAL, FILE-local, ordinary INSERT and analyzed EXPLAIN reach the intended gates. Remote FILE dispatch and S3/HDFS sink validation remain separate. Metadata local wildcard expectations are adapted; its independent exact-path and remote cases remain.
  9. Conditions and errors: the new sink-name condition selects only LOCAL and raises the shared explicit analysis error. Existing expression/statement exception boundaries preserve failure and its message; no silent continuation is added.
  10. Test coverage: inspected tests cover malformed/missing LOCAL properties, multiple formats, privileged rejection, FILE delegation, remote controls and distinct sink output positions. LOCAL-dependent success suites and local export-readback comparisons are deliberately retired; retained export completion/error, remote partition, UUID and ORC value checks remain in source.
  11. Expected results: no .out files change. The framework skips retired golden tags. ORC fixture footer column names and row counts match the new explicit projections and existing first golden counts (2 and 25,000); subsequent value assertions remain.
  12. Observability: the stable disabled analysis error is sufficient for deterministic admission rejection; no new operational logs or metrics are needed.
  13. Persistence and failover: no EditLog, replay, persisted metadata or transaction failover logic changes.
  14. Writes and atomicity: LOCAL sinks reject before execution setup. Remote write/transaction implementations are unchanged; test fixture deletion follows synchronous INSERT completion or failure.
  15. FE/BE variables: no new fields or transmission paths are introduced. Provider/endpoint/credential identity in the migrated fixtures and UUID export/reload is preserved through the existing storage-property path.
  16. Performance: rejection is constant-time and avoids later LOCAL parsing/discovery work; no new production hot-path scan or allocation is introduced.
  17. Other concerns and user focus: no additional user focus was supplied and no unresolved code candidate remains. Earlier sink-test, metadata-glob, account/UUID, complex-ORC and Azure-routing findings are fixed in their existing instances and are not reposted.

Validation limits: this was static review only. No builds, tests, Groovy compilation, Doris SQL execution or live Azure/S3 validation were performed. Retired LOCAL success/readback coverage is not claimed as retained runtime coverage. The review converged in one round.

Review converged after 1 round(s).

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 75.00% (3/4) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17616	5141	5038	5038
q2	2121	354	250	250
q3	10215	1372	814	814
q4	4683	459	339	339
q5	7433	797	527	527
q6	182	187	156	156
q7	741	818	589	589
q8	9260	1379	1053	1053
q9	5557	4194	4171	4171
q10	6810	1288	1033	1033
q11	434	237	234	234
q12	638	397	293	293
q13	18092	3662	2948	2948
q14	276	270	260	260
q15	q16	728	711	648	648
q17	1200	742	563	563
q18	6991	6196	6118	6118
q19	1104	957	588	588
q20	381	336	223	223
q21	5509	2479	2501	2479
q22	385	315	274	274
Total cold run time: 100356 ms
Total hot run time: 28598 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5939	5836	6117	5836
q2	723	594	540	540
q3	5509	5732	5234	5234
q4	2056	2120	1565	1565
q5	5832	5726	5923	5726
q6	254	190	139	139
q7	2226	2010	1829	1829
q8	2947	2623	2651	2623
q9	7898	8166	7824	7824
q10	3636	3567	3142	3142
q11	574	409	377	377
q12	656	661	498	498
q13	3288	3571	3034	3034
q14	288	290	262	262
q15	q16	678	675	610	610
q17	7698	6998	6882	6882
q18	12943	12224	12850	12224
q19	896	764	758	758
q20	2140	2178	1918	1918
q21	5520	4519	4627	4519
q22	500	443	380	380
Total cold run time: 72201 ms
Total hot run time: 65920 ms

@hello-stephen

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

query5	4336	601	441	441
query6	443	214	197	197
query7	4824	445	234	234
query8	316	165	153	153
query9	8790	3116	3126	3116
query10	508	296	274	274
query11	6006	3862	3566	3566
query12	150	93	87	87
query13	1247	447	341	341
query14	6164	3950	3888	3888
query14_1	3413	3414	3458	3414
query15	206	205	185	185
query16	959	455	418	418
query17	854	677	506	506
query18	2419	414	296	296
query19	188	163	128	128
query20	86	88	80	80
query21	208	131	103	103
query22	12878	13094	12885	12885
query23	13370	12937	12413	12413
query23_1	12417	12553	12598	12553
query24	7383	807	463	463
query24_1	466	488	467	467
query25	507	398	334	334
query26	1254	255	138	138
query27	2781	437	274	274
query28	4649	1959	1929	1929
query29	1518	573	448	448
query30	305	225	186	186
query31	996	931	805	805
query32	177	92	90	90
query33	532	299	231	231
query34	917	853	493	493
query35	719	751	650	650
query36	800	805	708	708
query37	135	107	82	82
query38	1774	1693	1647	1647
query39	794	794	749	749
query39_1	731	736	734	734
query40	217	116	93	93
query41	65	64	62	62
query42	86	87	82	82
query43	345	363	318	318
query44	1344	689	701	689
query45	171	182	160	160
query46	831	960	568	568
query47	2883	2877	2676	2676
query48	295	306	210	210
query49	566	400	300	300
query50	672	273	209	209
query51	10579	10677	10471	10471
query52	80	76	70	70
query53	187	210	148	148
query54	252	190	180	180
query55	68	66	65	65
query56	233	212	227	212
query57	1639	1626	1574	1574
query58	280	278	259	259
query59	2134	2134	1958	1958
query60	295	239	224	224
query61	148	151	148	148
query62	379	335	281	281
query63	189	160	154	154
query64	2736	925	773	773
query65	3336	3283	3282	3282
query66	1791	439	312	312
query67	20309	20098	19904	19904
query68	3056	1034	631	631
query69	404	290	262	262
query70	849	784	777	777
query71	294	242	229	229
query72	2838	2536	2311	2311
query73	528	536	298	298
query74	4637	4543	4345	4345
query75	2319	2284	1941	1941
query76	2303	1021	578	578
query77	353	380	268	268
query78	9728	9689	9172	9172
query79	909	859	522	522
query80	498	409	332	332
query81	525	327	270	270
query82	208	133	103	103
query83	310	206	186	186
query84	323	125	108	108
query85	845	425	379	379
query86	311	243	227	227
query87	2009	1936	1821	1821
query88	3641	2681	2661	2661
query89	316	276	236	236
query90	1856	171	166	166
query91	167	152	125	125
query92	101	89	89	89
query93	997	1028	572	572
query94	445	315	276	276
query95	574	398	298	298
query96	669	495	234	234
query97	2430	2434	2332	2332
query98	163	154	146	146
query99	760	754	662	662
Total cold run time: 233605 ms
Total hot run time: 151064 ms

@OIiveirra

Copy link
Copy Markdown
Collaborator Author

run external

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.16% (1/618) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17614	5181	5077	5077
q2	2142	312	257	257
q3	10218	1387	829	829
q4	4686	460	340	340
q5	7442	801	527	527
q6	180	192	155	155
q7	752	790	585	585
q8	9260	1308	1080	1080
q9	5556	4190	4199	4190
q10	6832	1285	1047	1047
q11	439	247	232	232
q12	637	392	277	277
q13	18114	3661	3027	3027
q14	278	269	250	250
q15	q16	718	715	671	671
q17	1189	760	581	581
q18	7077	6172	6133	6133
q19	1104	962	599	599
q20	378	337	237	237
q21	5630	2467	2412	2412
q22	386	332	270	270
Total cold run time: 100632 ms
Total hot run time: 28776 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5973	5887	6053	5887
q2	737	596	549	549
q3	5587	5739	5239	5239
q4	2045	2148	1550	1550
q5	5758	5674	5885	5674
q6	255	192	147	147
q7	2299	1991	1793	1793
q8	3067	2599	2648	2599
q9	7981	7743	7856	7743
q10	3607	3595	3139	3139
q11	572	402	374	374
q12	655	667	498	498
q13	3311	3589	2996	2996
q14	281	291	262	262
q15	q16	661	670	614	614
q17	7726	7028	6939	6939
q18	13042	12229	12909	12229
q19	885	785	774	774
q20	2134	2131	1940	1940
q21	5647	4620	4696	4620
q22	512	444	441	441
Total cold run time: 72735 ms
Total hot run time: 66007 ms

@hello-stephen

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

query5	4316	586	434	434
query6	431	208	191	191
query7	4803	443	227	227
query8	323	160	149	149
query9	8785	3091	3092	3091
query10	466	328	261	261
query11	5993	3865	3567	3567
query12	146	89	86	86
query13	1271	449	351	351
query14	6078	3879	3612	3612
query14_1	3393	3399	3390	3390
query15	217	195	177	177
query16	951	416	402	402
query17	840	624	513	513
query18	2416	418	295	295
query19	184	168	126	126
query20	84	81	81	81
query21	208	135	102	102
query22	12929	13003	12738	12738
query23	13474	12863	12282	12282
query23_1	12486	12615	12543	12543
query24	7392	821	467	467
query24_1	497	481	472	472
query25	541	425	385	385
query26	1263	262	147	147
query27	2753	440	275	275
query28	4626	1940	1948	1940
query29	1582	588	461	461
query30	304	222	187	187
query31	1017	951	818	818
query32	155	94	91	91
query33	555	295	247	247
query34	937	845	488	488
query35	739	747	648	648
query36	814	833	742	742
query37	127	115	80	80
query38	1779	1730	1669	1669
query39	795	809	742	742
query39_1	730	726	726	726
query40	220	123	101	101
query41	72	68	70	68
query42	92	88	82	82
query43	344	367	327	327
query44	1304	681	687	681
query45	180	183	171	171
query46	830	961	560	560
query47	2949	3000	2811	2811
query48	301	320	223	223
query49	580	407	295	295
query50	661	272	214	214
query51	10610	10481	10587	10481
query52	81	84	73	73
query53	224	204	153	153
query54	237	194	177	177
query55	71	65	61	61
query56	228	204	211	204
query57	1641	1623	1571	1571
query58	303	263	241	241
query59	2090	2202	2000	2000
query60	291	239	227	227
query61	146	142	142	142
query62	379	325	283	283
query63	181	151	159	151
query64	2715	917	741	741
query65	3324	3285	3267	3267
query66	1788	430	296	296
query67	20086	20208	20028	20028
query68	3037	1005	607	607
query69	393	291	245	245
query70	841	787	772	772
query71	299	230	207	207
query72	2767	2303	2272	2272
query73	506	516	293	293
query74	4614	4531	4372	4372
query75	2345	2311	1887	1887
query76	2316	1016	586	586
query77	365	363	255	255
query78	9990	9830	9148	9148
query79	1021	849	515	515
query80	527	418	344	344
query81	526	313	270	270
query82	863	132	110	110
query83	297	191	185	185
query84	312	126	100	100
query85	865	415	364	364
query86	313	241	230	230
query87	2020	1945	1811	1811
query88	3612	2682	2647	2647
query89	330	286	237	237
query90	1862	163	169	163
query91	157	150	127	127
query92	89	93	91	91
query93	1022	982	570	570
query94	441	346	264	264
query95	579	395	325	325
query96	658	473	234	234
query97	2443	2444	2349	2349
query98	175	169	146	146
query99	766	754	638	638
Total cold run time: 234462 ms
Total hot run time: 150575 ms

@hello-stephen

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

query1	0.01	0.01	0.00
query2	0.09	0.05	0.05
query3	0.26	0.10	0.09
query4	1.61	0.11	0.09
query5	0.23	0.24	0.24
query6	1.18	0.70	0.69
query7	0.04	0.01	0.00
query8	0.06	0.04	0.04
query9	0.41	0.35	0.34
query10	0.58	0.57	0.61
query11	0.20	0.14	0.14
query12	0.18	0.15	0.14
query13	0.49	0.49	0.50
query14	1.01	1.03	1.02
query15	0.63	0.61	0.63
query16	0.35	0.35	0.35
query17	1.12	1.14	1.13
query18	0.20	0.20	0.20
query19	2.13	2.02	2.01
query20	0.02	0.01	0.01
query21	15.47	0.20	0.14
query22	4.76	0.07	0.05
query23	16.28	0.34	0.12
query24	2.45	0.94	0.45
query25	0.11	0.05	0.07
query26	1.42	0.21	0.16
query27	0.06	0.04	0.03
query28	3.54	0.82	0.40
query29	12.50	4.13	3.25
query30	0.29	0.13	0.10
query31	2.78	0.55	0.34
query32	3.22	0.61	0.50
query33	3.16	3.19	3.21
query34	15.35	3.97	3.36
query35	3.35	3.36	3.35
query36	0.57	0.45	0.45
query37	0.09	0.07	0.06
query38	0.04	0.04	0.03
query39	0.04	0.03	0.03
query40	0.17	0.14	0.14
query41	0.10	0.02	0.02
query42	0.04	0.03	0.03
query43	0.04	0.03	0.04
Total cold run time: 96.63 s
Total hot run time: 24.49 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.16% (1/632) 🎉
Increment coverage report
Complete coverage report

yiguolei pushed a commit that referenced this pull request Oct 27, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: #69591, #69527

Problem Summary:

LOCAL TVF can discover files on a backend during analysis. Reject LOCAL
unconditionally before property parsing, backend selection and file
discovery. FILE TVF with local storage delegates to the same entry
point. Explicit LOCAL INSERT and analyzed EXPLAIN INSERT use the same
rejection before sink property validation. No configuration option
enables these paths, including for ADMIN/OPERATOR. The underlying
storage implementation is retained.

Tests assert rejection before backend access and preserve remote
FILE/sink behavior where supported. Existing LOCAL-dependent success
suites explicitly return while the feature is unavailable. Export
completion/error checks remain active; LOCAL export-readback comparisons
are retired. Remote HDFS/S3 partition cases, the existing UUID S3
fixture branch, and ORC value checks through S3 TVF using the original
uploaded fixtures remain active where present on this branch.

### Release note

LOCAL TVF and FILE TVF with local storage are disabled for all users.
There is no configuration option to re-enable them. Explicit LOCAL TVF
INSERT is also disabled.

### Check List (For Author)

- Test
- [x] Unit Test — 8 targeted FE tests passed; zero failures, errors or
skips.
- [x] Regression test — rejection coverage and affected-suite
adaptations included; suite bodies have not been executed locally.
    - [ ] Manual test
    - [ ] No need to test

Validation: Checkstyle passed for all 5 changed Java files. Changed
Groovy suites passed syntax parsing with Groovy 4.0.19. Whitespace
checks passed. Runtime regression and formal local code review have not
been performed.

- Behavior changed:
    - [ ] No.
- [x] Yes. LOCAL admission always fails; no startup opt-in is available.
- Does this need documentation?
    - [ ] No.
- [x] Yes. Release note included; operator documentation should reflect
disabled LOCAL support.

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
yiguolei pushed a commit that referenced this pull request Oct 27, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: #69591, #69528

Problem Summary:

LOCAL TVF can discover files on a backend during analysis. Reject LOCAL
unconditionally before property parsing, backend selection and file
discovery. FILE TVF with local storage delegates to the same entry
point. Explicit LOCAL INSERT and analyzed EXPLAIN INSERT use the same
rejection before sink property validation. No configuration option
enables these paths, including for ADMIN/OPERATOR. The underlying
storage implementation is retained.

Tests assert rejection before backend access and preserve remote
FILE/sink behavior where supported. Existing LOCAL-dependent success
suites explicitly return while the feature is unavailable. Export
completion/error checks remain active; LOCAL export-readback comparisons
are retired. Remote HDFS/S3 partition cases, the existing UUID S3
fixture branch, and ORC value checks through S3 TVF using the original
uploaded fixtures remain active where present on this branch.

### Release note

LOCAL TVF and FILE TVF with local storage are disabled for all users.
There is no configuration option to re-enable them. Explicit LOCAL TVF
INSERT is also disabled.

### Check List (For Author)

- Test
- [x] Unit Test — 8 targeted FE tests passed; zero failures, errors or
skips.
- [x] Regression test — rejection coverage and affected-suite
adaptations included; suite bodies have not been executed locally.
    - [ ] Manual test
    - [ ] No need to test

Validation: Checkstyle passed for all 5 changed Java files. Changed
Groovy suites passed syntax parsing with Groovy 4.0.19. Whitespace
checks passed. Runtime regression and formal local code review have not
been performed.

- Behavior changed:
    - [ ] No.
- [x] Yes. LOCAL admission always fails; no startup opt-in is available.
- Does this need documentation?
    - [ ] No.
- [x] Yes. Release note included; operator documentation should reflect
disabled LOCAL support.

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
@924060929
924060929 merged commit ba3d621 into apache:master Oct 27, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants