Repository navigation
[feature](fe) Support iceberg REST catalog labels as the $labels table - #69135
Merged
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
run buildall |
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed all 11 changed paths at 41f37f514bee86fb25738207246bf45e197a4751, including the REST label load, table wrappers, system-table binding, scan planning, serialized split and BE JNI consumer. No actionable issue was found. There were no existing inline findings to carry forward and no additional user focus points.
The reviewed execution chain is:
REST table load + labels -> statement/cache read table -> fixed labels schema and handles
-> ordered StaticDataTask rows -> existing serialized_split/JNI range
-> BE positional reader -> query predicates and aggregation
| Required checkpoint | Conclusion |
|---|---|
| Goal and evidence | REST catalogs expose object and field labels, including null names for dropped fields. Added unit assertions cover row content, projection, serialization, discovery and wrapper retention. |
| Scope and clarity | Changes are focused; shared projection and base-table resolution preserve the existing metadata-table logic. |
| Concurrency and locking | No new threads or locks. Existing statement memoization and atomic leases protect the same owners; label references are immutable. |
| Lifecycle and initialization | Labels survive all three read BaseTable rebuilds; existing resource cleanup and metadata freezing remain intact. Other rebuilds are write/procedure paths. No new resource cycle or static-order dependency. |
| Configuration | No new setting. Existing REST flavor and table-cache TTL/refresh policy govern availability and freshness. |
| Compatibility | No SPI, persisted-format or wire-field change. The split contains existing SDK classes and row values; the existing mixed-SDK-version restriction remains. |
| Thrift/protobuf IDs | Not applicable: no gensrc changes. |
| Parallel paths | Cached, direct, statement and authenticated read wrappers preserve labels; metadata schema/column overloads, existing metadata tables and native position-deletes routing were checked. |
| Conditions and errors | REST-only exposure, labels dispatch and the empty-snapshot exemption match the feature's semantics. Server-side scan planning remains explicitly rejected; residual predicates remain on BE. |
| Test coverage | New tests cover object/field/dropped-field/empty labels, reordered projection, serialization, REST-only metadata and cache/scope preservation. No direct labels planScan or SQL integration case is added. |
| Changed expected results | No regression result files changed. Unit expectations match the inspected schema and producer. |
| Observability | Existing load/planning error boundaries remain. No new persistent or distributed operation requires additional logging or metrics. |
| Transactions and persistence | Read-only catalog enrichment; no EditLog, replay, commit or failover state changes. |
| Writes and crash handling | No new data write. Existing lease and FileIO ownership remains unchanged. |
| FE-BE variables | Existing serialized_split and JNI format carry the task; no new protocol field or consumer class is required. |
| Performance | Linear label materialization into one split. The synthetic metadata file avoids a storage HEAD; ordered projection matches BE's positional reads. No new locking or table-data scan. |
| Additional edge cases | COUNT retains a readable slot and avoids base-table count pushdown; empty tables may expose labels; unknown statistics cannot become base-table cardinality. No unresolved static candidate remains. |
The SDK checks used the pinned Iceberg 1.12.0 sources: StaticDataTask projects in-memory rows without reading its synthetic file, and BaseTable keeps labels outside table metadata and serialization.
Validation limits: this was a static-only review. No builds or tests were run, and the author's reported results were not independently reproduced. The added tests do not exercise a REST-to-BE SQL query. All changed paths and identified risks received static coverage; there are no unresolved findings or uncovered changed paths.
Review converged after 1 round(s).
Contributor
TPC-H: Total hot run time: 28508 ms |
Contributor
TPC-DS: Total hot run time: 150765 ms |
Contributor
ClickBench: Total hot run time: 24.33 s |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
FE Regression Coverage ReportIncrement line coverage |
laskoviymishka
marked this pull request as ready for review
October 20, 2026 09:55
Contributor
|
Hi @laskoviymishka , LGTM, I will help to add some e2e tests later |
Contributor
Author
|
thanks @morningman ! |
morningman
added a commit
to laskoviymishka/doris
that referenced
this pull request
Oct 22, 2026
…m table ### What problem does this PR solve? Issue Number: apache#68916 Related PR: apache#69135 Problem Summary: t$labels was covered by unit tests only: nothing ran a REST table load that carries labels through FE planning and the BE JNI system-table scanner. No REST fixture returns labels yet (the upstream fixture option is apache/iceberg#18047), so the new suite test_iceberg_rest_labels starts a small HTTP proxy inside the regression runner, in front of the docker iceberg REST fixture. The proxy forwards every request unchanged and adds the suite's labels to load-table responses; the iceberg REST client, FE and BE run unmodified. It needs no docker change and no new image. The suite checks: - object labels, field labels and a nested field named by its full path (address.city), including a non-ASCII value; - another column order, a column subset, filters, and COUNT(*), which counts the labels (7) rather than the table's records (2); - a table with no snapshot still shows its labels; a table without labels, and the labelled table read without the proxy, show none; - a catalog with vended credentials, which has no table cache, shows the labels too; - after RENAME COLUMN a label follows the new name, after DROP COLUMN its field_name is NULL; FOR VERSION AS OF is ignored, because labels are not versioned, and names still come from the current schema; - the cached catalog keeps the loaded labels until REFRESH TABLE, while a catalog with meta.cache.iceberg.table.ttl-second = 0 sees new labels at once; - a hadoop catalog rejects t$labels with "Unknown sys table". ### Release note None ### Check List (For Author) - Test: Regression test - test_iceberg_rest_labels passes locally against the iceberg docker fixture, with FE built from this PR merged into master. - Mutation check: dropping the labels in IcebergTableCache.TableLease#snapshotReadTable, building the labels split without the requested projection, or offering t$labels on every catalog type each makes the suite fail. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
@laskoviymishka As mentioned, I added the e2e test. I pushed three commits on top of 41f37f5, without a force push, so your commit is untouched:
How the test gets labels. No REST fixture returns labels yet (apache/iceberg#18047 adds an option to the upstream one), so the suite starts a small HTTP proxy inside the regression runner, in front of the docker REST fixture. The proxy forwards every request unchanged and adds the suite's labels to load-table responses. Everything after the HTTP response is the real path: the iceberg 1.12 REST client parses the labels, FE plans the split, and BE reads it through What it checks.
Mutation check, one change at a time: dropping the labels in
For the doc PR: Local verification: the suite passes against the iceberg docker fixture with FE built from this PR merged into master (BE from a recent master build, since the PR has no BE change), and all 1646 iceberg connector FE unit tests pass on the same tree. |
Contributor
|
run buildall |
Contributor
TPC-H: Total hot run time: 28647 ms |
Contributor
TPC-DS: Total hot run time: 150440 ms |
Contributor
ClickBench: Total hot run time: 24.21 s |
Contributor
Author
|
@morningman awesome! thanks! |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: Review context preparation failed before Codex ran; inspect the 'Prepare authoritative PR context and required AGENTS guides' step. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
…em table ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: The Iceberg REST catalog spec now lets a catalog return `labels` with each table load (apache/iceberg#15750): catalog-owned, per-request metadata such as ownership, classification or cost attribution, at the table level (`object-labels`) and per field (keyed by `field-id`). Labels are not part of the table metadata, have no snapshots and are never written to `metadata.json`. Iceberg 1.12.0 exposes them on the loaded table via `SupportsLabels#labels()`. Doris had no way to show them. Add a `t$labels` system table for iceberg tables in a REST catalog: SELECT * FROM rest_ctl.db.tbl$labels; scope | field_id | field_name | key | value object | NULL | NULL | owner | team-a field | 2 | email | classification | pii One row per label. `field_name` is resolved against the current table schema and is NULL for a field id that is no longer in it (a dropped column). The schema mirrors iceberg's proposed `LabelsTable`. Implementation: - Keep labels when Doris rebuilds a loaded table. iceberg's REST client attaches them to the loaded `BaseTable` as a transient field; they are not table metadata, so a `BaseTable` rebuilt from its `TableOperations` must pass them explicitly. `IcebergTableCache.TableLease#snapshotReadTable` (cached reads), `IcebergStatementScope#snapshotReadTable` (statement reads) and `IcebergScanPlanProvider#wrapTableForScan` (Kerberos FileIO wrap) used constructors that reset labels to `Labels.EMPTY`, so every read saw no labels; they now pass `baseTable.labels()`. - `labels` is not a `MetadataTableType`, so it is handled next to the SDK metadata tables rather than through `MetadataTableUtils`. `IcebergConnectorMetadata` lists and resolves it only when the catalog flavor is `rest` (no other catalog returns labels), and serves its fixed schema and column handles without loading the base table. - `IcebergScanPlanProvider` plans it as one split. A new split-package helper `org.apache.iceberg.IcebergLabelsDataTask` builds an iceberg `StaticDataTask` (package-private in the SDK) holding the label rows, projected to the requested columns in BE's required-field order; its metadata `DataFile` is synthesized, so no storage request is made. The task is serialized like every other static metadata table, so BE's `IcebergSysTableJniScanner` reads it with no BE change. - The ordered system-table projection is extracted into `projectSystemTableSchema` and shared with the existing path, and the base-table load is split out of `resolveSysTable` as `resolveSysBaseTable`. - Labels are not versioned: like `$snapshots`/`$history`, the snapshot pin of the handle is ignored, and `$labels` is exempt from the resolved-empty-snapshot fence, so a table with no commit still shows its labels. Labels reflect the table load the statement used, so they follow the iceberg table cache (`meta.cache.iceberg.table.ttl-second`); REFRESH TABLE picks up new labels. Like the other system tables, a REST catalog that requires server-side scan planning rejects `$labels`. ### Release note Support querying REST catalog labels of an iceberg table through the `tbl$labels` system table. ### Check List (For Author) - Test: Unit Test - Added IcebergLabelsDataTaskTest, the $labels cases in IcebergConnectorMetadataSysTableTest, IcebergTableCacheTest#snapshotReadTableKeepsCatalogLabels and IcebergStatementScopeTest#sharedTableKeepsCatalogLabels. - `build.sh --fe` (Maven build cache disabled) passes on top of current master. Ran every iceberg and hudi FE unit test with run-fe-ut.sh: 1990 tests, 0 failures, 5 skipped. - Regression test pending a REST fixture that returns labels (apache/iceberg#18047). - Behavior changed: No (adds a new system table for REST catalogs only) - Does this need documentation: Yes (doc PR to follow) Co-authored-by: Isaac <no-reply@databricks.com>
…m table ### What problem does this PR solve? Issue Number: apache#68916 Related PR: apache#69135 Problem Summary: t$labels was covered by unit tests only: nothing ran a REST table load that carries labels through FE planning and the BE JNI system-table scanner. No REST fixture returns labels yet (the upstream fixture option is apache/iceberg#18047), so the new suite test_iceberg_rest_labels starts a small HTTP proxy inside the regression runner, in front of the docker iceberg REST fixture. The proxy forwards every request unchanged and adds the suite's labels to load-table responses; the iceberg REST client, FE and BE run unmodified. It needs no docker change and no new image. The suite checks: - object labels, field labels and a nested field named by its full path (address.city), including a non-ASCII value; - another column order, a column subset, filters, and COUNT(*), which counts the labels (7) rather than the table's records (2); - a table with no snapshot still shows its labels; a table without labels, and the labelled table read without the proxy, show none; - a catalog with vended credentials, which has no table cache, shows the labels too; - after RENAME COLUMN a label follows the new name, after DROP COLUMN its field_name is NULL; FOR VERSION AS OF is ignored, because labels are not versioned, and names still come from the current schema; - the cached catalog keeps the loaded labels until REFRESH TABLE, while a catalog with meta.cache.iceberg.table.ttl-second = 0 sees new labels at once; - a hadoop catalog rejects t$labels with "Unknown sys table". ### Release note None ### Check List (For Author) - Test: Regression test - test_iceberg_rest_labels passes locally against the iceberg docker fixture, with FE built from this PR merged into master. - Mutation check: dropping the labels in IcebergTableCache.TableLease#snapshotReadTable, building the labels split without the requested projection, or offering t$labels on every catalog type each makes the suite fail. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
morningman
force-pushed
the
iceberg-1.12-bump
branch
from
October 27, 2026 02:52
253e13a to
cdfb6c4
Compare
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Reviewed all 13 changed files and the upstream/downstream read path at cdfb6c4. No actionable defect was found. There are no existing inline findings to retain, and no additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: the REST-only t$labels view follows base-handle resolution, fixed schema/column binding, the statement's loaded table, materialized label rows, and the existing JNI metadata reader. Unit and regression cases exercise the intended semantics in source; their execution was not verified in this review.
- Scope and clarity: changes stay within the Iceberg connector, its tests, and a narrow ImportControl exception. The extracted ordered projection retains the existing metadata-table dependency handling and avoids a second implementation.
- Concurrency: production code adds no worker, lock, or mutable shared container. Labels are read from the statement/cache table generation. The regression proxy uses a concurrent map and shuts down its executor in finally; existing production lock order is unchanged.
- Lifecycle: cached, direct, and authenticated read-table rebuilds preserve transient labels. Existing statement leases and resource cleanup remain in control; the serialized task holds materialized scalar rows rather than a table, FileIO, or authenticator reference.
- Configuration: no configuration item is added. Existing table-cache TTL, zero-TTL reads, and REFRESH invalidation govern label freshness as described.
- Compatibility: no shared SPI or storage format changes. Both connector and JNI scanner use Iceberg 1.12.0, and the task consists of existing SDK classes. The SDK constructor and row projection match this use (StaticDataTask source). Existing restrictions when FE and BE carry different Iceberg serialization versions remain; rolling-upgrade execution was not tested.
- Thrift/Protobuf IDs: neither directory is changed; no field IDs or enum values are introduced.
- Parallel paths: cache-backed and direct/vended statement reads preserve labels, as does the authenticated scan wrapper. Other BaseTable wrappers serve writes or procedures and do not feed labels reads. Existing metadata and native position-delete paths retain their dispatch.
- Conditional checks: REST-only discovery, fixed-schema dispatch, and the labels-specific empty-snapshot exception are consistent. Ordinary reads retain their snapshot fence. Labels intentionally ignore the selected snapshot and resolve field names against the loaded current schema. Server-side scan-planning rejection remains enforced.
- Test coverage: inspected unit cases for discovery, schema/handles, row construction, projection, dropped fields, empty labels, serialization and table-wrapper preservation. The regression suite covers filters, COUNT, nested names, Unicode, an empty base table, schema evolution, time travel, cache/REFRESH, vended reads, and non-REST rejection. Builds, tests, formatting checks and runtime validation were not run under the static-only contract.
- Expected results: all 17 result queries have matching output blocks; row counts, column widths/order, null values and cache transitions are internally consistent with the fixture. Output-generation provenance and actual execution cannot be established from static inspection.
- Observability: existing query/JNI exception and profile paths cover this metadata view. No additional critical metric or log requirement was identified.
- Persistence and failover: labels remain transient catalog response data; no edit-log, replay, or persistent state transition is introduced.
- Writes and atomicity: production changes are read-only. No transaction, commit, data-write, or crash-recovery behavior is modified.
- FE-to-BE values: the existing serialized-split route carries ordered projected rows. The synthetic zero-size metadata file is internal to the task; the JNI reader iterates rows without opening that file. System handles bypass base-table metadata COUNT and streaming, and predicates remain for BE evaluation.
- Performance: label materialization and serialization are linear in returned labels, use one split, and avoid a storage HEAD on the dummy file. No new heavy work under a production lock or obvious performance regression was found.
- Core invariants and other checks: no committed-data visibility, visible-version, delete-bitmap, or BE allocation path is changed. Unsupported label sources and missing requested fields fail explicitly. System-table scans are excluded from SQL result caching, avoiding reuse based on unrelated data-table metadata versions. Connector boundaries and the narrow style suppression remain consistent with the repository's architecture.
The review converged after two rounds, including a separate risk challenge and a final pass over the unchanged payload. No unresolved candidate or static coverage gap remains. Validation is limited to source inspection and review-artifact consistency checks; no build, test, benchmark, or FE/BE runtime result is claimed.
Review converged after 2 round(s).
Contributor
|
run buildall |
morningman
approved these changes
Oct 27, 2026
Contributor
TPC-H: Total hot run time: 28416 ms |
Contributor
TPC-DS: Total hot run time: 151295 ms |
Contributor
ClickBench: Total hot run time: 24.36 s |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
FE Regression Coverage ReportIncrement line coverage |
924060929
approved these changes
Oct 27, 2026
6 of 10 tasks
morningman
added a commit
to apache/doris-website
that referenced
this pull request
Oct 27, 2026
Document the Iceberg REST Catalog `$labels` system table introduced by apache/doris#69135, available since Doris 5.0.0. Update the English and Chinese current-version Iceberg Catalog guides with the field definitions, query examples and expected results, cache refresh instructions, and restrictions. Cover REST-only availability, nested fields, schema changes, empty results, snapshot-independent labels, and the server-side scan planning restriction. Add support-matrix and system-table version notes. ## Validation - Docusaurus MDX compilation, front matter, and section-link targets passed for both pages. - Internal-link checks passed for both pages; existing external links are report-only. - All five SQL and expected-output blocks match between English and Chinese. - `git diff --check` passed. - SEO checking reported existing duplicate-title warnings across versions and locales; front matter is unchanged. - AI self-review found no additional issues. Page paths, navigation, and site code are unchanged. Older versions are not updated because this feature starts in 5.0.0. ## Versions - [x] dev - [ ] 4.x - [ ] 3.x - [ ] 2.1 or older (not covered by version/language sync gate) ## Languages - [x] Chinese - [x] English ## Docs Checklist - [x] Checked by AI - [ ] Test Cases Built (documentation only; no runtime test cases added or run) - [x] Updated required version and language counterparts, or explained why not - [x] If only one language changed, confirmed whether source/translation counterparts need sync (not applicable: both languages updated) Co-authored-by: morningman <moringman@apache.org>
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: #68916
Related PR: #69031
Problem Summary: The Iceberg REST catalog spec now lets a catalog return
labelswith each table load (apache/iceberg#15750): catalog-owned, per-request metadata such as ownership, classification or cost attribution, at the table level (object-labels) and per field (keyed byfield-id). Labels are not part of the table metadata, have no snapshots and are never written tometadata.json. Iceberg 1.12.0 exposes them on the loaded table viaSupportsLabels#labels(). Doris had no way to show them.Add a
t$labelssystem table for iceberg tables in a REST catalog:One row per label.
field_nameis resolved against the current table schema and is NULL for a field id that is no longer in it (a dropped column). The schema mirrors iceberg's proposedLabelsTable.Implementation:
BaseTableas a transient field; they are not table metadata, so aBaseTablerebuilt from itsTableOperationsmust pass them explicitly.IcebergTableCache.TableLease#snapshotReadTable(cached reads),IcebergStatementScope#snapshotReadTable(statement reads) andIcebergScanPlanProvider#wrapTableForScan(Kerberos FileIO wrap) used constructors that reset labels toLabels.EMPTY, so every read saw no labels; they now passbaseTable.labels().labelsis not aMetadataTableType, so it is handled next to the SDK metadata tables rather than throughMetadataTableUtils.IcebergConnectorMetadatalists and resolves it only when the catalog flavor isrest(no other catalog returns labels), and serves its fixed schema and column handles without loading the base table.IcebergScanPlanProviderplans it as one split. A new split-package helperorg.apache.iceberg.IcebergLabelsDataTaskbuilds an icebergStaticDataTask(package-private in the SDK) holding the label rows, projected to the requested columns in BE's required-field order; its metadataDataFileis synthesized, so no storage request is made. The task is serialized like every other static metadata table, so BE'sIcebergSysTableJniScannerreads it with no BE change.projectSystemTableSchemaand shared with the existing path, and the base-table load is split out ofresolveSysTableasresolveSysBaseTable.$snapshots/$history, the snapshot pin of the handle is ignored, and$labelsis exempt from the resolved-empty-snapshot fence, so a table with no commit still shows its labels.Labels reflect the table load the statement used, so they follow the iceberg table cache (
meta.cache.iceberg.table.ttl-second); REFRESH TABLE picks up new labels. Like the other system tables, a REST catalog that requires server-side scan planning rejects$labels.Release note
Support querying REST catalog labels of an iceberg table through the
tbl$labelssystem table.Check List (For Author)
build.sh --fe(Maven build cache disabled) passes on top of current master. Ran every iceberg and hudi FE unit test with run-fe-ut.sh: 1990 tests, 0 failures, 5 skipped.test_iceberg_rest_labels(external_table_p0/iceberg). The docker REST fixture returns no labels (the upstream fixture option is OpenAPI: emit labels from the REST server test fixture iceberg#18047), so the suite starts a small HTTP proxy inside the regression runner, in front of that fixture, which adds labels to load-table responses; the iceberg REST client, FE planning and the BE JNI scanner run unmodified, and no docker change is needed. It covers object, field and nested-field labels, column order and subsets, filters and COUNT, a table with no snapshot, RENAME/DROP COLUMN,FOR VERSION AS OF, the table cache and REFRESH TABLE, a vended-credentials catalog, and the rejection on a non-REST catalog.What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)