morningman commented on PR #69135: URL: https://github.com/apache/doris/pull/69135#issuecomment-6341268838
@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: 1. **Merge of apache/master.** #69382, merged today, renamed `IcebergScanPlanProvider#requestedLowerNames` to `requestedColumnNames`. CI builds the PR merged into master, and there the new `doPlanLabelsSystemTableScan` no longer compiled: `cannot find symbol: method requestedLowerNames`. 2. **`[fix](fe) Project $labels with requestedColumnNames after the master rename`**: the one-line follow-up. The labels schema is all lower case, so the projected schema is unchanged. 3. **`[test](regression) Add an e2e test for the iceberg REST $labels system table`**: the new suite `test_iceberg_rest_labels`. **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 `IcebergSysTableJniScanner`. No docker change and no new image. **What it checks.** - object, field and nested-field labels (`address.city`), including a non-ASCII value - another column order, a column subset, filters, and `COUNT(*)`, which returns 7 labels on a 2-row table - a table with no snapshot shows its labels; a table without labels, and the labelled table read without the proxy, show none - a vended-credentials catalog, which has no table cache, shows the labels - after `RENAME COLUMN` a label follows the new name, after `DROP COLUMN` its `field_name` is NULL; `FOR VERSION AS OF` is ignored and names still come from the current schema - the cached catalog keeps the loaded labels until `REFRESH TABLE`; with `meta.cache.iceberg.table.ttl-second = 0` new labels show up at once - a hadoop catalog rejects `t$labels` with `Unknown sys table` **Mutation check**, one change at a time: dropping the labels in `IcebergTableCache.TableLease#snapshotReadTable`, building the split without the requested projection, or offering `$labels` on every catalog type each makes the suite fail. Two parts of the PR cannot be reached from SQL today, so only the unit tests cover them: - `IcebergStatementScope#snapshotReadTable`. Inside a statement the table comes from a cache lease (cache on) or the tracked raw table (cache off, e.g. vended credentials); this rebuild only runs outside a statement scope, or without a resource tracker as in the unit test. - The `$labels` exemption from the resolved-empty-snapshot fence. A system-table scan never gets a query-begin snapshot pin, and an explicit `FOR VERSION/TIME AS OF` resolves to a real snapshot, so the fence never applies to `$labels`. **For the doc PR:** `key` is a reserved word in Doris, so the column has to be quoted: ``SELECT `key`, value FROM tbl$labels``. 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
