Skip to content

Honor auto-create-schema settings for tree and table LOAD - #19128

Open
Caideyipi wants to merge 7 commits into
masterfrom
codex/table-load-auto-create-schema
Open

Caideyipi wants to merge 7 commits into
masterfrom
codex/table-load-auto-create-schema

Conversation

@Caideyipi

@Caideyipi Caideyipi commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Description

LOAD can create missing schema even when auto-creation is disabled by the request or session. Honor the combined global, request, and session policy across tree and table LOAD paths while preserving shared client session state.

For tree LOAD, preserve the policy through native schema validation, mini-file conversion, Analyzer and scheduler fallbacks, normal timeseries creation, template extension, and template activation. For table LOAD, preserve it through native schema validation, mini-file conversion, Analyzer and scheduler fallbacks, and tablet rewrite. Asynchronous handoff retains the effective policy.

Native LOAD now checks this policy before creating missing databases, preventing empty-database side effects when auto-creation is disabled. A localized missing-database error bypasses tablet conversion. Existing databases and schema remain usable. Preserve the default enabled behavior and existing TAG-order, type-validation, partial-insert, and ordinary INSERT behavior.

Validation

  • Clean 28-module source-reactor compilation of production and test sources passed for English and Chinese locales with build-cache reuse disabled, using JDK 17.0.15 and Maven 3.8.8.
  • Latest results across 54 unique selected unit methods in five classes: 54 passed, 0 failures/errors/skips, including all four database-policy/error-preservation regressions added in the review follow-up. The English run initially passed 53/54; after updating one existing schema-read fixture, only that failed method was rerun in the clean Chinese-locale reactor and passed.
  • Final Spotless apply with the integration-test profile, Checkstyle, English forbidden-API checks, and the diff whitespace check passed. Duplicate Spotless/forbidden-API checks were skipped in the Chinese run.
  • RAT was skipped. Used the published TsFile dependency 2.5.1-261023-20261023.065606-1 without rebuilding it. The full test suite and integration tests were not rerun in this follow-up. Remote CI was not awaited.

This PR has:

  • completed independent re-review at the latest head.
  • added or modified tests covering the affected paths.

@codecov

codecov Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.50467% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.73%. Comparing base (2075032) to head (0de7dcb).

Files with missing lines Patch % Lines
...ryengine/plan/analyze/load/LoadTsFileAnalyzer.java 51.72% 14 Missing ⚠️
...ngine/plan/analyze/schema/NormalSchemaFetcher.java 71.42% 4 Missing ⚠️
...l/metadata/fetcher/TableHeaderSchemaValidator.java 0.00% 3 Missing ⚠️
.../plan/analyze/load/LoadTsFileTableSchemaCache.java 88.88% 1 Missing ⚠️
...n/planner/plan/node/load/LoadSingleTsFileNode.java 80.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19128      +/-   ##
============================================
+ Coverage     48.53%   48.73%   +0.20%     
  Complexity      635      635              
============================================
  Files          5573     5574       +1     
  Lines        412215   412268      +53     
  Branches      54338    54366      +28     
============================================
+ Hits         200059   200919     +860     
+ Misses       212156   211349     -807     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Caideyipi
Caideyipi marked this pull request as draft November 5, 2026 04:00
@Caideyipi Caideyipi changed the title Honor auto-create-schema settings for table-model LOAD Honor auto-create-schema settings for tree and table LOAD Nov 5, 2026
@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Initial independent self-review — tree LOAD only. Table behavior was excluded as requested.

Reviewed SHA: 0c083585227d8fcb91042a9c85d36e42687820bc
Final local SHA: 5e846cf7f2948acf024c9c5256d634c8dc3b37e1
Confirmed source-branch and PR-head SHA: 5e846cf7f2948acf024c9c5256d634c8dc3b37e1

P2 — Native tree LOAD could ignore a disabled session auto-create policy (fixed). With global and LOAD auto-creation enabled, a session disabling auto-creation could still reach schema creation. The per-file dialect switch rebuilt SessionInfo with the flag defaulting to true, and the list-schema validation path did not apply the effective policy guard. The fix preserves the session flag and client version and guards list-schema auto-creation. A real-TsFile regression runs the complete analysis entry point, verifies read-only schema fetching, forbids the auto-create fetcher, and checks every switched session retains the disabled flag.

No other actionable findings were identified in the scoped tree review. This was one review round; the repaired commit has not received a second independent review in this session.

Verification:

  • Clean 28-module source-reactor compilation with build-cache reuse disabled; JDK 17.0.15 and Maven 3.8.8. Five focused tree-policy methods passed. The initial new regression fixture stopped before the final schema-cache flush and failed; after correcting the fixture, only that method was rerun and passed. Latest results across the six unique selected methods: 6 passed, 0 failures/errors/skips.
  • Checkstyle, Spotless, and the final diff whitespace check passed. A published timestamped TsFile dependency (2.5.1-261023-20261023.065606-1) was used rather than rebuilt.
  • Verification completed locally after remote setup/dependency-resolution attempts were stopped. RAT was skipped; the full test suite, integration tests, and the zh-locale build were not rerun.
  • New-head CI was queued/running at a single snapshot and was not awaited.

The fix is committed and pushed, and both remote heads were confirmed. The PR remains draft because this round found a behavioral issue; a fresh independent tree-only review of the full current tree diff and relevant context is required before marking it ready.

@Caideyipi
Caideyipi marked this pull request as ready for review November 5, 2026 09:15
@Caideyipi

Copy link
Copy Markdown
Collaborator Author

Independent re-review — tree LOAD auto-creation.

Reviewed SHA: 5e846cf7f2948acf024c9c5256d634c8dc3b37e1
Final local SHA: 5e846cf7f2948acf024c9c5256d634c8dc3b37e1
Confirmed source-branch and PR-head SHA: 5e846cf7f2948acf024c9c5256d634c8dc3b37e1

Findings: No actionable issues found in the full current tree LOAD diff and relevant context, including the preceding session-preservation and native list-schema fixes. The review covered native schema validation, mini-file conversion, Analyzer and scheduler fallback, asynchronous policy handoff, normal timeseries and template creation, and the related unit tests and Scan-to-Query regression. Table behavior was excluded as requested and retains its prior acceptance.

Fixes: None needed. No source changes or new commits were made in this round.

Verification and limits:

  • Source review and git diff --check passed. This was one complete independent re-review round.
  • No builds or tests were rerun in this read-only round. The preceding round reported six selected unit methods passing locally at this same SHA. The full test suite, integration tests, and local zh-locale build were not rerun; RAT was skipped in the preceding verification.
  • At one CI snapshot, the current-head zh-locale check had passed, while other checks were queued or running. CI was not awaited, and incomplete checks are not reported as passed.

The actual source branch and PR head were confirmed to match the reviewed revision immediately before marking the entire PR ready for review. The resulting state was verified as open, non-draft. No update steps remain incomplete.

@luoluoyuyu

Copy link
Copy Markdown
Member

Database auto-creation is still not covered by the policy.

Table LOAD: LoadTsFileTableSchemaCache.createTableAndDatabaseIfNecessary() (L322) calls autoCreateTableDatabaseIfAbsent() whenever needToCreateDatabase, and that method (L353) only checks the global config — neither allowAutoCreateSchema nor the session flag. With global enabled + 'auto-create-schema'='false' (or enableAutoCreateSchema=false) + a missing database, LOAD still runs CREATE DATABASE, then fails table validation, leaving an empty database behind.
Tree LOAD: TreeSchemaAutoCreatorAndVerifier.autoCreateDatabase() (L276) runs whenever isAutoCreateDatabase, which defaults to the global config and ignores both the request attribute and the session flag.
Async load: ActiveLoadTsFileLoader.loadTsFile() sets autoCreateDatabase from the global config as well.
LoadTsFileAnalyzer.isAutoCreateSchemaEnabled (L152) is still global && request, so the session flag never reaches the autoCreateDatabase decision. Suggest computing one effective policy (global && request && session) in the analyzer and using it for database/table/column creation, and throwing a clear "database does not exist, auto-create-schema disabled" error instead of creating it. Current tests all set setAutoCreateDatabase(false) and the IT pre-creates the database, so this path is untested.

@Caideyipi
Caideyipi marked this pull request as draft November 6, 2026 02:39
@Caideyipi
Caideyipi marked this pull request as ready for review November 6, 2026 02:40
@Caideyipi

Copy link
Copy Markdown
Collaborator Author

@luoluoyuyu, addressed the database-policy comment in 0de7dcb04b1.

  • Both analyzer constructors now compute the effective global && request && session policy. Native tree/table schema validation and asynchronous handoff use it.
  • Tree LOAD checks existing databases before rejecting a missing database under the disabled policy. Table LOAD checks the same policy before CREATE DATABASE. Existing databases remain usable, and the active loader's default database-creation flag cannot override a disabled request policy.
  • Added a localized missing-database error and a specific exception that bypasses tablet conversion, preserving that error instead of replacing it with a conversion failure.
  • Added four regressions for request/session/global restrictions and error preservation. The database tests keep database creation enabled and verify that neither the database executor nor Coordinator performs a write. Existing table tests now exercise the database path, and async tests include the global switch.

Verification:

  • Clean 28-module source-reactor compilation of production and test sources passed for both English and Chinese locales, with build-cache reuse disabled, using JDK 17.0.15 and Maven 3.8.8.
  • The English run passed 53 of 54 selected methods, including all four new regressions. One existing fixture disabled verification while still expecting a schema read; it now verifies matching existing schema. Only that failed method was rerun in the clean Chinese-locale reactor and passed. Latest results across all 54 unique selected methods in five classes: 54 passed, 0 failures/errors/skips.
  • Final Spotless apply with the integration-test profile, Checkstyle, English forbidden-API checks, and the diff whitespace check passed. Duplicate Spotless/forbidden-API checks were skipped in the Chinese run.
  • RAT was skipped. Used the published TsFile dependency 2.5.1-261023-20261023.065606-1 without rebuilding it. The full suite and integration tests were not rerun.
  • New-head CI was queued/running at one snapshot and was not awaited.

For the Codecov report, this update adds targeted missing-database and conversion-error coverage. The report predates this follow-up; no updated coverage percentage is claimed here.

  • Reviewed SHA (comment scope): bbe9ae4
  • Final local SHA: 0de7dcb
  • Confirmed source-branch and PR-head SHA: 0de7dcb

The fixes are committed and pushed. Independent re-review of the full updated PR remains pending. The PR remains open and non-draft, and discussions have been left unresolved.

@sonarqubecloud

sonarqubecloud Bot commented Nov 6, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

3 participants