Repository navigation
[fix](fe) Keep sibling capabilities on iceberg-on-HMS tables pinned at a snapshot - #69999
Merged
morningman merged 1 commit intoNov 3, 2026
Merged
Conversation
…t a snapshot ### What problem does this PR solve? Issue Number: None Related PR: apache#69299, apache#69488 Problem Summary: An Iceberg table stored in HMS can be read through a dedicated Iceberg catalog or through an HMS catalog ('type'='hms'), whose embedded Iceberg connector builds the table's schema. fe-core resolves the table-scoped capabilities (Top-N lazy materialization, nested column pruning, field-id access paths, storage predicate pruning, nested column DDL, write-only defaults, auto analyze) from the catalog's connector or from the per-table set in the table's schema, never from the embedded connector. So the Hive connector copies the ones Iceberg declares into the per-table set of every schema it delegates, but it did that only in getTableSchema(session, handle), not in getTableSchema(session, handle, snapshot). fe-core uses the second one for explicit time travel and, since apache#69299, for every statement that reads an Iceberg table: the latest-snapshot pin retains its schema, and the table's capabilities are then read from that schema. On an iceberg-on-HMS table such a statement therefore saw none of the inherited capabilities. Reproduced on a local cluster with a standalone Hive 3.1.2 metastore, comparing the same statements on twin format-version 3 tables, one through each catalog: | Statement | Iceberg catalog | HMS catalog before | HMS catalog after | |---|---|---|---| | EXPLAIN ... ORDER BY id LIMIT 1 (latest, and FOR VERSION AS OF) | VMaterializeNode | no lazy materialization | VMaterializeNode | | EXPLAIN SELECT struct_element(s, 'a2') | all access paths: [s.a2] | no pruning | all access paths: [s.a2] | | UPDATE t SET c = 100 WHERE d = DEFAULT(d) | updates the rows with d = 42 | updates nothing | updates the rows with d = 42 | | MERGE ... ON t.id = s.id AND t.d = DEFAULT(t.d) | updates row 0 | updates nothing | updates row 0 | | DELETE FROM t WHERE d = DEFAULT(d) | error: cannot be used in a DELETE | deletes nothing | same error | | SELECT DEFAULT(d) FROM t | error: must reference the written table | NULL | same error | The snapshot overload now builds the delegated schema through the same helper as the latest-schema overload, which resolves the owning sibling once, asks it for the schema (at the pin, for the snapshot overload) and reflects the inherited capabilities onto it. A hudi-on-HMS table is unchanged: the Hudi connector declares no capabilities. ### Release note Fix iceberg-on-HMS tables read through an HMS catalog losing Iceberg optimizations (Top-N lazy materialization, nested column pruning) and write-default resolution of DEFAULT(column) in UPDATE / MERGE conditions. ### Check List (For Author) - Test: Unit Test / Regression test / Manual test - HiveConnectorMetadataSiblingDelegationTest: the per-table capability cases now check both getTableSchema overloads; a new case checks that the snapshot overload forwards the pin unchanged and resolves the owner once. All 49 fe-connector-hive test classes pass (502 tests). Three mutations (no reflection in the snapshot overload, asking the sibling for its latest schema, resolving the owner twice) each fail only their target cases. - New regression test_iceberg_on_hms_gateway_pinned_capabilities (hive3 docker). Run locally against a standalone Hive 3.1.2 metastore with a file:// warehouse (catalog properties adapted): fails on the HMS catalog's first Top-N check without this change, passes with it. - Behavior changed: Yes. Statements on iceberg-on-HMS tables read through an HMS catalog plan with the same capabilities as through an Iceberg catalog. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <[email protected]>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 28433 ms |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed PR 69999 at the exact bundle/live head 6d547c0. The change keeps sibling capabilities on both latest and pinned snapshot schemas by forwarding the owning handle and snapshot through one shared helper, while preserving schema columns, properties, and write metadata identity. I traced the FE pinned-schema publication path, Iceberg schema-id selection and applySnapshot flow, Hive Iceberg/Hudi owner routing, per-table capability resolution, statement-scoped metadata memoization, and the parallel Hive-handle path. The changed unit tests and the new Iceberg/HMS gateway regression suite cover both overloads, explicit time travel, nested field-id access, Top-N lazy materialization, write-only defaults, and negative cases; no concrete defect was verified. All three required independent review passes reported NO_NEW_VALUABLE_FINDINGS, and RISK-1 through RISK-3 were dismissed with evidence. No existing inline review threads or blocking findings apply, and there was no additional user focus. This was a static review; builds and tests were not executed per the review contract. The review converged in round 1.
Review converged after 1 round(s).
Contributor
TPC-DS: Total hot run time: 150940 ms |
Contributor
ClickBench: Total hot run time: 24.22 s |
924060929
approved these changes
Nov 3, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
Iceberg tables accessed through an HMS catalog need their owning connector's table-scoped capabilities on both latest and pinned schemas. This change applies the existing capability reflection through one shared delegation helper.
The independent static review found no blocking defects in the FE changes or the necessary FE/BE scan contracts. Snapshot forwarding, schema fields, write metadata identity, statement-scoped metadata reuse, and Hive/Hudi/system-table boundaries remain consistent. The added tests cover both overloads, capability filtering, Top-N, nested-field reads, and write-default behavior.
APPROVE for head 6d547c0. No builds or tests were run during this review. The supplied CI snapshot reports successful P0, external regression, FE UT, and compilation checks, but FE coverage failed; merge eligibility requires a separate check.
Automated FE-focused static review (gpt-6.1-sol, high). No build or tests were run.
Contributor
|
PR approved by anyone and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
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 problem does this PR solve?
Issue Number: None
Related PR: #69299, #69488
Problem Summary:
In short
An Iceberg table stored in HMS and read through an HMS catalog (
'type'='hms') loses every capability it inherits from the embedded Iceberg connector in any statement that pins the table at a snapshot. Since #69299 that is every statement that reads it: it gets no Top-N lazy materialization and no nested column pruning, andDEFAULT(column)in the conditions ofUPDATEandMERGEsilently matches nothing. This PR makes the snapshot overload ofHiveConnectorMetadata#getTableSchemapass the inherited capabilities on, as the latest-schema overload already does. It is a master-only fix (branch-4.x has no connector SPI), left over from #69488 (review comment 4355508100).Background
fe-core resolves a table-scoped capability (
PluginDrivenExternalTable.hasCapability) from the catalog's connector or from the per-table set in the table's schema. It never asks the embedded connector that built the schema of an iceberg-on-HMS table. SoHiveConnectorMetadatacopies the capabilities that fe-core resolves per table (SIBLING_INHERITABLE_CAPABILITIES: auto and sampled analyze, Top-N lazy materialization, nested column pruning, field-id access paths, storage predicate pruning, nested column DDL and write-only defaults) from the owning connector into the per-table set of each schema it delegates. Iceberg declares seven of the eight; it does not declare sampled analyze.It did this only in
getTableSchema(session, handle).getTableSchema(session, handle, snapshot)returned the sibling's schema unchanged. fe-core uses that overload for explicit time travel. Since #69299 it also uses it for the latest-snapshot pin of every statement that reads an Iceberg table: the pin retains its schema (retainSchema(true)),PluginDrivenMvccExternalTablebuilds the schema at that pin, andgetSchemaCacheValue(), wherehasCapabilityreads the per-table set, returns it for the rest of the statement.The problem, and what it cost
Reproduced on a local cluster with a standalone Hive 3.1.2 metastore. Two identical format-version 3 tables
(id INT, c INT, s STRUCT<a:INT, b:STRING>)get two rows, thenADD COLUMN d INT DEFAULT 42,RENAME COLUMN s.a TO a2and a third row withd = 7. The same statements then run against one table through the Iceberg catalog and against the other through the HMS catalog.EXPLAIN SELECT * FROM t ORDER BY id LIMIT 1, also withFOR VERSION AS OFVMaterializeNodeVMaterializeNodeEXPLAIN SELECT struct_element(s, 'a2') FROM tall access paths: [s.a2]all access paths: [s.a2]UPDATE t SET c = 100 WHERE d = DEFAULT(d)MERGE INTO t USING (SELECT 0 AS id) s ON t.id = s.id AND t.d = DEFAULT(t.d) WHEN MATCHED THEN UPDATE SET c = 300DELETE FROM t WHERE d = DEFAULT(d)cannot be used in a DELETESELECT DEFAULT(d) FROM tmust reference a column of the table that this statement writesThe
UPDATE,MERGEandDELETErows fail silently: the statement succeeds and changes nothing. Reads of the renamed field return the right values in all three columns: on master the HMS catalog does not prune the struct at all, and with this PR it prunes it with field-id access paths, as the Iceberg catalog does.INSERT ... VALUES (DEFAULT(d))andINSERT ... SELECT DEFAULT(d)already wrote 42 through both catalogs.How this PR fixes it
Both overloads now go through one private helper,
delegatedTableSchema. It resolves the owning sibling once, asks it for the schema (at the pin, for the snapshot overload), and passes the result through the existingreflectSiblingCapabilities. The snapshot reaches the sibling unchanged. A hudi-on-HMS table is unaffected: the Hudi connector declares no capabilities, so its schema comes back as it was. A plain Hive table never reaches this branch.Results
Through the HMS catalog, every statement in the table above now plans and answers as it does through the Iceberg catalog.
Release note
Fix Iceberg tables read through an HMS catalog losing Top-N lazy materialization and nested column pruning, and
DEFAULT(column)inUPDATE/MERGEconditions on them reading NULL instead of the column's write-default.Check List (For Author)
HiveConnectorMetadataSiblingDelegationTestthe per-table capability cases now check bothgetTableSchemaoverloads, and a new case checks that the snapshot overload forwards the pin unchanged and resolves the owner once. All 49 fe-connector-hive test classes pass (502 tests); fe-connector-hive Checkstyle 0 violations. Three mutations (the snapshot overload without the reflection, asking the sibling for its latest schema, resolving the owner a second time) each fail only their target cases.test_iceberg_on_hms_gateway_pinned_capabilities(hive3 docker) runs the probe above through both catalogs; its.outholds both results under paired tags. I ran it locally against a standalone Hive 3.1.2 metastore with afile://warehouse, with the catalog properties adapted, not against the hive3 docker environment: without this change it fails on the HMS catalog's first Top-N check, with it it passes.DEFAULT(column)on them resolves as it does there.