[#12407] fix(lance): Hydrate empty schema before table alteration - #12895
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The refresh logic change is targeted, aligns with the stated bug scenario, and is backed by updated documentation plus regression tests covering the key recovery and no-op cases.
Pull request overview
This PR fixes Lance table schema refresh behavior in the lakehouse-generic catalog so that tables with empty stored columns can recover their schema from the underlying Lance dataset even when lance.version is already recorded, preventing tables from getting “stuck” with empty columns.
Changes:
- Updates
loadTablerefresh logic to always re-examine the Lance dataset when stored columns are empty (in bothDECLARED_AND_EMPTYandVERSION_CHECKflows). - Avoids redundant metadata writes when the dataset is still empty and the recorded version is unchanged.
- Updates documentation and expands regression tests to cover recovery, no-op rechecks for truly empty datasets, and fallback behavior when dataset open fails.
File summaries
| File | Description |
|---|---|
| docs/lakehouse-generic-lance-table.md | Updates schema refresh mode and zero-column dataset behavior documentation to match the new recheck/repair logic. |
| catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java | Adjusts schema refresh gating so empty stored columns trigger dataset schema re-examination and avoids no-op metadata updates for unchanged empty datasets. |
| catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/TestLanceTableOperations.java | Adds/updates regression tests for repairing empty stored columns with a recorded version, avoiding redundant updates for truly empty datasets, and preserving version on fallback. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ts own Repairing an empty column list on every load is correct but costly: in the default DECLARED_AND_EMPTY mode any table with no stored columns opened its Lance dataset on every loadTable, and a genuinely empty dataset whose version moved rewrote its metadata each time. The reason lance.version could not be trusted as a "checked, really empty" marker is that alterTable records it after any dataset change without reading the schema, so a table registered with no columns ends up carrying a version its column metadata never came from. Record the confirmation separately instead: recordCheckedEmptyVersion now also writes lance.empty-schema-checked-version, and only a schema read that returned no columns ever writes it. A load skips the dataset when the marker still matches lance.version. Any path that records a newer version invalidates the marker without knowing about it, columns read from the dataset clear it, and version metadata that cannot be parsed confirms nothing. Tables already stuck with an unconfirmed empty column list carry no marker, so they are re-examined once, repaired, and then stop paying for the check. Both refresh modes now share one isDatasetReadNeeded predicate; the only difference left between them is whether the dataset version is also consulted. Tests: a confirmed-empty table opens no dataset, a repaired table stops opening one, a marker left behind by an older version is ignored, and unparsable version metadata is treated as unconfirmed. Claude-Session: https://claude.ai/code/session_01DZwZThqoBB7NojLLpdYPfX
Code Coverage Report
Files
|
yuqi1129
changed the title
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change directly addresses the inconsistent-state bug described, keeps normal loadTable behavior intact, and is backed by targeted tests for both success and failure paths.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
jerryshao
approved these changes
Sep 4, 2026
yuqi1129
added a commit
that referenced
this pull request
Sep 4, 2026
… before table alteration (#12895) (#12926) **Cherry-pick Information:** - Original commit: 1db4894 - Target branch: `branch-1.3` - Status: ✅ **Conflicts resolved** Resolved the test-source conflict and verified the backport with `:catalogs:catalog-lakehouse-generic:test`. --------- Co-authored-by: Qi Yu <yuqi@datastrato.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Why are the changes needed?
Registering an existing Lance table accepts an empty column list and defers schema hydration. alterTable previously used the metadata-only parent load method, so an index alteration could update the Lance dataset and persist its latest version while the Gravitino columns remained empty. A later default load would interpret that version as confirmation of a genuinely zero-column schema and skip the dataset read.
This patch prevents that concrete inconsistent state. It was found while investigating #12407, but the issue does not show that an alter operation caused the reported UI symptom, so the API and authorization path still needs separate investigation.
Related: #12407
Does this PR introduce any user-facing change?
No API or configuration key is added. For a table with empty stored columns, alterTable now performs a schema read first and fails without changing the dataset if that read cannot complete. Ordinary loadTable behavior is unchanged.
How was this patch tested?
Added tests covering:
Ran: