Static review of #69593 at 7221734. One P2 finding: the existing BindTVFTableSinkTest does not opt into the new default-off gate and now fails before its output-position assertions. No confirmed P0/P1 findings and no existing inline threads to carry forward. No additional user review focus was supplied.

Review converged after 3 rounds: all three reviewers returned NO_NEW_VALUABLE_FINDINGS on the final comment set, all 13 changed files and applicable upstream/downstream paths were covered, and every candidate and initial risk has an explicit disposition.

Critical checkpoint conclusions:

1. Goal and proof: the named LOCAL read, FILE selecting LocalProperties, and explicit LOCAL INSERT/EXPLAIN paths reach admission before local discovery or sink execution. New unit/regression assertions cover these paths statically; the existing sink test regression is reported inline.
2. Focus: one startup configuration and a shared admission helper keep the production change small; test and pipeline opt-ins support the intended behavior.
3. Concurrency: no new threads or shared mutable runtime structures. Existing privilege read locks remain short, with no new IO or lock-order change.
4. Lifecycle: the primitive config initializer has no dependency cycle. Superclass fields are initialized before LOCAL constructor admission. Its own discovery is blocked on denial, and denied local sinks do not reach coordinator execution. SELECT-source discovery can precede sink admission.
5. Configuration: mutable defaults to false and ConfigBase rejects runtime updates. Every serving FE must load the opt-in and restart; seven regression configurations explicitly enable it.
6. Compatibility: default-disable is an intentional behavior change. Mixed FE settings can produce different admission until every FE is configured. No storage-format or function-signature change.
7. Thrift/proto: none changed; release-branch field/enum comparisons are not applicable.
8. Parallel paths: legacy/Nereids reads, FILE-local delegation, ordinary/Lance reads, and explicit LOCAL INSERT/EXPLAIN were traced. Later resource USAGE checks remain. INSERT also retains its existing ADMIN/LOAD check, so OPERATOR alone is insufficient for execution without LOAD.
9. Conditions: disabled-first and ADMIN/OPERATOR ordering are clear. Name normalization precedes the sink gate; all explicit LOCAL formats share it. No optimizer-expression or nullable-column change.
10. Tests: new constructor tests cover disabled/enabled and denied/ADMIN/OPERATOR cases; binder tests cover disabled/enabled/denied and remote noninterference; the integration suite covers LOCAL/FILE SELECT and EXPLAIN errors. The existing fixture requires opt-in as reported. Actual command-run privilege intersection and default-off cluster integration remain test coverage gaps; the modified pipeline configs enable the feature.
11. Results: no generated result files changed. New regression assertions expect errors and do not need ordered output. Builds/tests were not run, as required by this static-only contract.
12. Observability: rejection errors identify the flag/restart remedy or required privilege. No new distributed operation warrants additional metrics or logging.
13. Transactions/persistence: no EditLog, commit/visibility, delete bitmap, storage version, or failover-state change.
14. Writes/crashes: denied local sinks fail during analysis before writes. Existing writer/coordinator cleanup is unchanged; no new crash, leak, atomicity, or deadlock mechanism was found.
15. FE/BE variables: the flag is FE admission only; no wire variable or serialization send path was added.
16. Performance/memory: admission adds constant-cost flag/privilege checks, with no new scan, large allocation, copy, or IO.
17. Other risks: eager constructor auth, resource authorization, view expansion, catalog-function caching, and effective storage routing were rechecked. No additional substantiated defect within the stated mitigation scope.

Coverage limits: this is a static review, with no compilation, tests, runtime validation, or submodule initialization. CI deployment orchestration outside this checkout was not verified. The PR mitigates its named entry points; pre-existing effective local IO through other Hadoop/provider, metadata, direct remote-named Lance, or custom JNI writer paths remains outside this protection. This is not evidence of a universal local-IO sandbox. No unresolved static candidate remains within the stated PR scope.

Review converged after 3 round(s).

Static review of all 14 changed files at db2422e found no new actionable defect. The existing P2 comment 4372016395 is fixed: BindTVFTableSinkTest now saves, enables and restores enable_local_tvf while retaining its ROOT context and output-position assertions. No existing P0/P1 finding remains to carry forward. No additional user review focus was supplied.

Critical checkpoint conclusions:

1. Goal and evidence: The false default rejects explicit LOCAL reads, FILE-local delegation and LOCAL sink binding before their backend discovery or destination processing. Added unit tests cover disabled/enabled admission, unauthorized access and retained validation; their execution was outside this static review.
2. Scope and clarity: One startup configuration and shared checkAccess helper serve the read constructor/auth override and sink binding. The remaining changes support fixtures and regression deployment opt-in.
3. Concurrency and locks: No new threads, shared collections or lock order are introduced. Configuration is startup-only; existing bounded privilege lookups add no IO under authorization locks.
4. Lifecycle and initialization: FE configuration loads before services. The early constructor check uses initialized superclass state; later authentication preserves the parsed resource check. No new ownership or static-initialization dependency was found. Test configuration and static mocks are restored/closed.
5. Configuration: enable_local_tvf defaults false and ConfField is nonmutable by default; ConfigBase rejects runtime mutation. Every query-serving FE requires explicit configuration and restart, matching the description and release note. Seven regression deployment configurations opt in, including cloud_p1 overrides.
6. Compatibility and upgrades: Default admission changes intentionally. Function signatures, storage formats and persisted metadata are unchanged; an all-FE rollout is needed for consistent admission across FEs.
7. Thrift/protobuf: No definitions or protocol IDs changed; release-branch field comparison is inapplicable.
8. Parallel paths: Reviewed direct LOCAL, generic FILE-local, factory/legacy/DESCRIBE construction, ordinary/analyzed EXPLAIN and LOCAL INSERT. Remote FILE and remote sink branches retain existing behavior. TVFs disable SQL caching; prepared execution binds fresh functions. Parsed-only EXPLAIN performs no discovery. The parquet metadata wildcard helper inherits the LOCAL gate it invokes.
9. Conditions and authorization: The disabled check precedes global ADMIN/OPERATOR admission. Later checkAuth still calls the existing resource USAGE check, including FILE delegation. INSERT retains its existing ADMIN/LOAD execution prerequisite. No concrete admission bypass was found in the reviewed paths.
10. Test coverage: JUnit cases inspect CSV/Lance, LOCAL/FILE-local, ADMIN/OPERATOR success, unauthorized denial, INSERT/EXPLAIN and remote validation. The regression suite checks SELECT/EXPLAIN aliases against configured admission; existing enabled read/write regressions remain opted in. Backend mocks match the checked-out eligibility policy, and separate test-class JVMs prevent cross-class flag coupling.
11. Expected results: No .out results changed. New error assertions match checked-out error text and config result indexing; no nondeterministic output assertion was introduced.
12. Observability: Rejections identify the opt-in/restart action or ADMIN/OPERATOR privilege. Existing backend error context remains; this synchronous admission check adds no distributed operation requiring new logs or metrics.
13. Transactions and persistence: No EditLog, transaction state, master-failover behavior or visible-version handling changed.
14. Writes and crashes: The LOCAL sink gate runs before sink translation and dispatch. Existing write, coordinator cleanup and crash/atomicity behavior remain unchanged; no new resource or lock lifecycle was added.
15. FE/BE variables: No new transmitted variable or sending path; admission stays in FE and existing sink file-type encoding is unchanged.
16. Performance: Constant-time admission precedes existing property/backend/IO work. No new repeated scan, significant allocation or heavy locked work was found.
17. Other invariants: Committed-data visibility, version consistency, MoW delete bitmaps and BE memory accounting are untouched. Analysis failures propagate as failures. Further checks of construction, cached plans, resource authentication and fixture/deployment routing found no additional issue.

The review converged in two rounds, including a final pass over the frozen payload. No unresolved code candidate or static coverage gap remains within this PR's admission-change scope. Validation is static only: no builds, unit/regression tests, live file operations or mixed-FE rollout were executed. Enabled-path sandbox hardening and unrelated provider/metadata access policies remain separate work, as stated in the PR.

Review converged after 2 round(s).

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

Static review of b92f0eb found one P2 issue: the existing parquet-meta regression still expects local wildcard success and the former glob error, but both now reach the unconditional LOCAL rejection. See the inline finding. No P0/P1 issue was confirmed. Existing thread 4372016395 is resolved on this head: the duplicate-output sink test now uses S3 and retains its ExprId assertions; no duplicate is posted. No additional user focus was supplied.

Critical checkpoint conclusions:

1. Goal and proof: LOCAL/FILE-local reads reject before local parsing/backend discovery; explicit LOCAL INSERT and analyzed EXPLAIN reject before sink-property validation. New tests cover these paths statically; the remaining parquet-meta test adaptation is the finding.
2. Scope: two admission calls and a shared rejection helper are focused; the broader suite edits follow disabled LOCAL dependencies.
3. Concurrency: no new threads, shared mutable gate state, locks, heavy locked work or lock-order changes.
4. Lifecycle: no new resource ownership, initialization-order dependency or cleanup obligation; rejection precedes local backend resources.
5. Configuration: none added; no startup or dynamic opt-in re-enables LOCAL.
6. Compatibility: intentional SQL behavior break is described in the PR release note. No symbol, stored format or wire change; older FEs keep their prior behavior until upgraded.
7. Thrift/protobuf: untouched; cross-branch field/enum checks are inapplicable.
8. Parallel paths: traced Nereids/legacy factories, FILE delegation, INSERT/EXPLAIN and parquet-meta glob delegation. Remote S3/HDFS delegates and sinks remain outside the gate; literal-path parquet metadata is a separate unchanged API.
9. Conditions: the normalized LOCAL sink-name check and shared helper explain the early rejection; privileges cannot bypass it.
10. Test coverage: constructor tests cover invalid properties, FILE, formats including Lance, privileged users and no backend access; sink tests cover INSERT/EXPLAIN and remote validation. Rejection regression coverage exists; the untouched parquet-meta wildcard assertions need adaptation.
11. Expected results: no .out edits. The ORC fixtures contain 2 and 25000 rows with matching target schemas; COUNT replacements preserve existing scalar snapshots and retained array-value/map-count checks. The two parquet-meta expectations are stale.
12. Observability/errors: the disabled analysis error reaches existing exception boundaries; no new distributed operation requires metrics or extra logging.
13. Transactions/persistence: no commit, version, EditLog or failover-state changes.
14. Writes/atomicity: LOCAL sinks fail before coordinator execution or destination writes; remote writes are unchanged. Replacement Stream Loads use existing success/row-count/publication checks and sync.
15. FE/BE variables: no new values or send paths.
16. Performance: constant-time rejection avoids local discovery/RPC; no new loops, caches or large allocations.
17. Other invariants: no changed rowset visibility, delete bitmap, committed-data or BE memory logic; no additional verified issue.

Coverage: all 28 changed paths and related invocation/framework paths reviewed by the main agent, two complete-scope reviewers and an additional risk reviewer. Converged after 2 rounds; all latest-round reviewers returned NO_NEW_VALUABLE_FINDINGS on the frozen payload, with every candidate/risk disposed and no unresolved static coverage question.

Validation limits: static inspection only; no builds, unit/regression runs or submodule initialization. LOCAL positive scanner suites and local export readbacks are intentionally retired, reducing scanner/export-content coverage; retained export completion/error checks do not prove exported bytes. Runtime behavior remains unverified.

Review converged after 2 round(s).

Reviewed all 29 changed files at 6280fc56e387223f15ee497d50e7ba27f6042e92, including related callers, test expectations and the existing review threads. No new substantiated defects were found.

Checkpoint conclusions:

1. Goal and evidence: LOCAL construction rejects before common-property parsing, backend selection and file discovery; FILE with LocalProperties delegates to the same gate. Explicit LOCAL INSERT and analyzed EXPLAIN INSERT reject during sink binding before sink-property validation. New FE tests and the account regression define rejection coverage; tests were inspected, not executed.
2. Scope: production changes share one unconditional rejection helper between two admission points; regression changes adapt consumers of the disabled feature.
3. Concurrency and locks: the gates run synchronously during FE analysis, add no shared state, threads, I/O or lock acquisition, and preserve planner cleanup on exceptions. No new lock-order or deadlock issue was found.
4. Lifecycle and memory: rejected construction publishes no usable LOCAL object or backend future; rejected sinks never create a coordinator. The helper adds no owned buffers, static initialization dependencies or resource lifecycle. Test static mocks are closed.
5. Configuration: no configuration is introduced. LOCAL is intentionally unavailable to every user, including ADMIN/OPERATOR, with no opt-in.
6. Compatibility: LOCAL SQL behavior intentionally changes to rejection on upgraded FEs. There is no changed storage format, durable representation or FE/BE wire contract.
7. Thrift/protobuf: no definitions, field IDs or enum values change; cross-release definition comparison is inapplicable.
8. Parallel paths: checked LOCAL/FILE-local SELECT, registry/DESCRIBE, analyzed EXPLAIN, explicit LOCAL sinks, forwarded requests, SQL cache and prepared execution. No gate bypass was substantiated. PARSED_PLAN only prints syntax. Remote FILE delegates and S3/HDFS sink validation remain outside the gate. PARQUET_META literal-local paths remain available; local wildcard expansion rejects through LOCAL.
9. Conditions and errors: sink names are normalized before the LOCAL comparison, and wrappers/query/RPC boundaries preserve failure and the disabled reason. No swallowed exception or continuation into local file access was found.
10. Test coverage: inspected rejection cases cover empty/invalid properties, CSV/JSON/Lance, privileged users, FILE-local delegation, SELECT/EXPLAIN and sink analysis, plus remote dispatch/validation checks. Live INSERT execution is covered here by FE analysis tests rather than a new runtime regression.
11. Expected results: unchanged ORC fixtures and golden counts align with the Stream Load replacements; default load checks validate success, row counts and publication. Retained golden tags remain reachable after removed readback tags. No generated result file changes.
12. Observability: the existing query error/logging path reports the deterministic disabled reason; the admission-only change introduces no distributed operation requiring new metrics or tracing.
13. Transactions and persistence: no EditLog, replay, failover, visible-version, delete-bitmap or transaction-state changes.
14. Writes and crashes: LOCAL sinks fail before physical sink creation, deletion, coordinator registration or BE writes. Existing remote execution and atomicity behavior are unchanged by these gates.
15. FE/BE variables: none are added or require propagation through alternate send paths.
16. Performance: rejection avoids discovery/RPC work; supported sink analysis adds a constant-time name check. No new redundant scans, allocations or hot-path loop was found.
17. Additional risks: rechecked alternate callers, uninvoked datatype LOCAL helpers, suite skips, cleanup and retained remote cases. All recorded risk questions and candidate concerns were dismissed with evidence; none remains unresolved.

Existing comments 4372016395 and 4373703267 are fixed on this head: the duplicate-output binding fixture now uses S3, and both PARQUET_META local-wildcard cases expect rejection. Neither was reposted. There are no existing P0/P1 findings to carry forward. No additional user focus was supplied.

Coverage limits: this was a static review; no builds or tests were run. Distributed runtime behavior is unverified. LOCAL-dependent suites are skipped and local export-content comparisons are retired by this PR; export completion/error checks and retained remote cases were reviewed. These are validation limits, with no uncovered changed-file scope or unresolved source candidate. All three reviewers returned NO_NEW_VALUABLE_FINDINGS in round 1.

Review converged after 1 round(s).

Reviewed pinned head 63b2f2a and all 29 changed files (5 Java, 24 Groovy). One new P2 finding: both ORC fixture migrations introduce an unconditional AWS S3 upload that fails for the regression framework's supported Azure configuration. No new P0/P1 production defect was verified.

The two existing P2 threads are fixed on this head: 4372016395's output-position unit test now binds an S3 sink, and 4373703267's matching/no-match metadata wildcard cases both expect LOCAL rejection. Neither is reposted; there are no applicable existing P0/P1 inline findings to carry. No additional user review focus was supplied.

Critical checkpoint conclusions from static inspection:

1. Goal and evidence: direct LOCAL and FILE-local construction reject before common property parsing, backend selection, glob/schema RPC or Lance work. LOCAL INSERT and analyzed EXPLAIN INSERT reject during BindSink before sink property validation and writer creation. Constructor, privilege, format, FILE delegation and sink/EXPLAIN negative tests cover these contracts in source; tests were not executed.
2. Scope: two production admission points share one unconditional rejection helper. The wider diff adapts dependent tests and deliberately retains underlying storage implementations.
3. Concurrency and locks: no shared mutable state, threads or locks are added. Rejection runs synchronously during analysis; no new lock ordering or heavy work under locks is introduced.
4. Lifecycle and memory: superclass initialization only creates per-object containers/options. Rejection precedes backend/coordinator acquisition; no new ownership cycles, tracked native allocations or static initialization dependencies are introduced.
5. Configuration: no option is added or can re-enable LOCAL, including for ADMIN/OPERATOR; dynamic propagation is inapplicable.
6. Compatibility: rejecting previously accepted LOCAL SQL is the intended behavior change and is described in the PR release note. Wire formats, persisted representations and symbols are unchanged. The Azure regression compatibility issue is reported inline.
7. Thrift/protobuf: no definitions change; cross-release ID/enum comparison is inapplicable.
8. Parallel paths: Nereids and legacy LOCAL construction, FILE-local, analyzed EXPLAIN and explicit LOCAL sinks reach the guard. Remote FILE delegates and S3/HDFS sink branches remain available. Explicit literal-local parquet_meta paths are intentionally retained; local wildcard expansion delegates to the rejected LOCAL constructor.
9. Conditions: the sink-name condition is normalized and restricts only LOCAL. The helper explains why rejection precedes parsing/discovery; no privilege or format bypass remains in the reviewed entry points.
10. Test coverage: all changed Java tests and Groovy bodies were read. The negative LOCAL/FILE and sink matrices, retained remote partition/export/metadata branches, and output-position test cover the intended changes by inspection. RISK-1 identifies the provider gap in the migrated data tests.
11. Expected results: unchanged ORC footer schemas contain the selected fields; row counts are 2 and 25000. Existing scalar snapshots match the COUNT replacements, and the harness skips unused removed tags while consuming repeated qt_sql tags sequentially. No .out file was changed or regenerated.
12. Observability and error handling: LOCAL-disabled errors propagate through existing analysis/request exception boundaries. No new distributed operation requires new logs or metrics.
13. Transactions and persistence: no edit-log, replay, transaction or version-visibility implementation changes. Delete-bitmap and committed-data visibility invariants are unaffected by these admission changes.
14. Data writes and atomicity: LOCAL sink rejection precedes physical writer/coordinator execution. Fixture uploads use unique keys, synchronous INSERT and finally cleanup; no source-read lifetime race was found. Azure upload failure remains the reported exception.
15. FE/BE propagation: no transmitted variables or sending paths change; rejected LOCAL plans do not reach execution.
16. Performance: constant-time rejection avoids parsing, scheduling and file discovery. No production hot-path loops, repeated scans or unbounded allocations are introduced.
17. Other concerns and scope limits: all initial risks and candidate points are accepted, deduplicated or dismissed with source evidence. LOCAL-only suites and export readback comparisons are intentionally retired, reducing that coverage; export completion/error checks and independent remote cases remain active.

Review converged after 2 rounds: all three reviewers returned NO_NEW_VALUABLE_FINDINGS on the unchanged final payload, with full assigned static coverage and no unresolved candidate. No builds, tests, backend RPCs or real object-store operations were performed. Remote runtime correctness and the execution status of the inspected tests are not established by this review.

Review converged after 2 round(s).

Static review of all 29 changed files at 0350472 found no new verified findings. Both complete-scope reviewers and the separate admission/fixture risk reviewer returned NO_NEW_VALUABLE_FINDINGS; all candidates and risk focuses have evidence-based dispositions. The review converged in one round.

Critical checkpoint conclusions:

1. Goal and evidence: LOCAL construction rejects before its property parsing, backend selection, file discovery or schema access. FILE with local storage delegates to that constructor. LOCAL INSERT and analyzed EXPLAIN INSERT reach the same rejection in BindSink before sink property validation or physical writer creation. The new Java and account regression tests cover these admission mechanisms statically; no test execution is claimed.
2. Scope and clarity: the two production files add small gates sharing one unconditional denial method. The remaining changes adapt tests to the deliberate loss of LOCAL support and retain original ORC data checks.
3. Concurrency and locks: no new threads, shared mutable production state, lock acquisitions or lock-order changes. The denial precedes backend/coordinator work; fixture keys use UUIDs to isolate concurrent suites.
4. Lifecycle and initialization: superclass initialization creates empty collections/default fields; rejected construction opens no file or RPC handle. Static Mockito mocks close through teardown or try-with-resources. Fixture cleanup follows the synchronous INSERT call, whose executor waits for backend execution before returning.
5. Configuration: no new configuration or opt-in is introduced; ADMIN/OPERATOR cannot enable LOCAL. No dynamic propagation or restart-sensitive state is added.
6. Compatibility: the intentional incompatibility is SQL admission denial, described in the PR release note. No storage format or existing function symbol is removed, and no FE/BE serialization migration is required. Enforcement belongs to the FE executing each request during rollout.
7. Thrift/protobuf: no definitions changed, so cross-release field/enum-ID comparison is inapplicable.
8. Parallel paths: reviewed direct factory calls, Nereids LOCAL/FILE wrappers, sink binding and analyzed EXPLAIN. Remote FILE dispatch and S3/HDFS sink validation remain independent. Local parquet wildcard expansion delegates to LOCAL and now expects rejection; literal parquet metadata retains its preexisting separate path. EXPLAIN PARSED PLAN stops before analysis and performs no IO.
9. Conditional checks: the sink name is normalized before comparison; the common denial method documents why admission precedes parsing/discovery/binding. Remote property validation remains reachable. Unauthorized ordinary INSERT can fail its existing privilege check earlier, without local writes.
10. Test coverage: new negative cases cover empty/invalid properties, csv/json/lance, FILE-local delegation, privileged callers, INSERT/EXPLAIN binding and remote controls. Existing LOCAL-only success suites are explicitly retired. Export completion/error checks, independent S3 view readback and HDFS/S3 partition cases remain. Repository-wide residual LOCAL helpers in nested-type suites are uncalled.
11. Expected results: no generated .out files changed. The migrated original ORC fixtures have root fields matching the explicit projections: k1/k2/c_decimal (2 rows) and id/m (25000 rows). Replacement COUNT assertions preserve the first golden values and subsequent array/map assertions. The framework skips retired unique golden tags while preserving the retained repeated-tag sequence.
12. Observability and errors: the explicit denial survives wrapper and statement-executor exception boundaries. No silent continuation, new distributed operation, or additional logging/metrics requirement was found.
13. Transactions and persistence: no EditLog, metadata, visible-version or failover state changes. No delete-bitmap/rowset logic is modified; LOCAL admission fails during analysis.
14. Data writes and atomicity: rejected LOCAL sinks cannot reach physical translation or coordination. The test fixture migrations use existing synchronous OLAP ingestion, unique remote keys and finally cleanup; original target schemas and data assertions remain.
15. FE/BE variables: none added; no scattered thrift sending paths require updates.
16. Performance and memory: successful remote binding adds one constant name comparison; rejected reads avoid local RPC work. No new hot-path loop, significant allocation, BE memory ownership, nullable-column or resource-cycle issue is introduced.
17. Additional risk sweep: checked provider detection, Azure placeholder and explicit endpoint handling, SDK/auth consistency, original ORC footer metadata, ingestion/cleanup ordering, golden tags, remaining callers and retained suite setup/cleanup. No unresolved static candidate or coverage gap remains.

Existing discussions were independently revalidated and are addressed at this head: sink unit test now uses S3 while retaining distinct-output assertions; parquet local globs both expect the disabled error; Azure fixture upload now uses Azure Blob upload/deletion and passes the matching resolved endpoint/provider to the TVF. No duplicate inline findings are added. No existing P0/P1 was found to carry forward.

User focus: no additional focus points were supplied. Validation limits: this was a static review with source/control-flow, fixture-metadata and golden-output inspection; builds, tests, Doris runtime execution and real-cloud upload/read validation were not performed. Convergence describes completion of that static scope.

Review converged after 1 round(s).