github-actions[bot] commented on code in PR #65867:
URL: https://github.com/apache/doris/pull/65867#discussion_r3683702461
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -958,6 +964,245 @@ public Map<String, String> getScanNodeProperties(
return props;
}
+ /**
+ * Build the Paimon table object that is serialized to the BE.
+ *
+ * <p>Every table loaded from a metastore-backed Paimon catalog (HMS /
DLF) carries a Paimon
+ * {@code HiveCatalogLoader} in its {@link CatalogEnvironment}. The BE
only reads — via
+ * FE-resolved splits and the object store — and never needs the catalog,
yet deserializing that
+ * loader forces the whole Hive metastore stack onto the BE classpath:
{@code HiveConf}, the
+ * metastore API, and, when a system table resolves its latest snapshot,
even the metastore
+ * client (DLF's {@code ProxyMetaStoreClient} and its REST stack). So we
serialize a catalog-less
+ * table to the BE:
+ * <ul>
+ * <li>data table: drop the catalog loader. A {@link FileStoreTable} is
fully defined by
+ * fileIO / location / schema / catalogEnvironment, and its dynamic
options (time travel,
+ * incremental) are merged into the schema by {@code copy(...)} —
which
+ * {@link #resolveScanTable} has already applied — so rebuilding from
+ * fileIO / location / schema preserves everything except the
catalog loader.</li>
+ * <li>system table (e.g. {@code $snapshots}): rebuild it over a
catalog-less data table so
+ * {@code SnapshotManager#latestSnapshotId} lists the snapshot
directory on the filesystem
+ * instead of calling the metastore. The base table is the one the
FE-side wrapper was
+ * built over ({@link PaimonTableHandle#getSysBaseTable()}), and for
the system tables that
+ * pick their snapshot on the BE ({@link
PaimonScanParams#resolvesSnapshotOnBackend}) what
+ * the catalog would have done there is done here instead: see
+ * {@link #authorizeDeferredScan} and {@link #pinCatalogSnapshot}.
Every other system table
+ * reads what the FE already planned, so it is handed over
untouched. The relation-scoped
+ * scan params {@link #resolveScanTable} applied to the original
wrapper are re-applied to
+ * the rebuilt one by {@link #reapplyScanParams}.</li>
+ * </ul>
+ */
+ // Package-private for direct unit testing (PaimonBackendBoundTableTest).
+ Table tableForBackend(PaimonTableHandle handle, Table scanTable) {
+ if (scanTable instanceof FileStoreTable) {
+ // resolveScanTable's copy(...) merged the relation's dynamic
options into the schema,
+ // and the rebuild below goes through that schema, so this branch
needs no re-application.
+ return dropCatalogLoader((FileStoreTable) scanTable);
+ }
+ if (!handle.isSystemTable()) {
+ return scanTable;
+ }
+ // The very same base table the FE-side wrapper was built over, so
that the BE never sees a
+ // different schema generation than the one this query was planned
with.
+ FileStoreTable dataTable = handle.getSysBaseTable();
+ if (dataTable == null) {
+ return scanTable;
+ }
+ String sysTableType = handle.getSysTableName();
+ boolean resolvesOnBackend =
PaimonScanParams.resolvesSnapshotOnBackend(sysTableType);
+ if (PAIMON_FILES_SYSTEM_TABLE.equalsIgnoreCase(sysTableType)) {
+ authorizeDeferredScan(dataTable);
+ }
+ FileStoreTable baseForBackend = dropCatalogLoader(dataTable);
+ if (resolvesOnBackend) {
+ baseForBackend = pinCatalogSnapshot(baseForBackend, dataTable);
+ }
+ Table catalogLessSysTable = SystemTableLoader.load(sysTableType,
baseForBackend);
+ if (catalogLessSysTable == null) {
+ return scanTable;
+ }
+ return reapplyScanParams(catalogLessSysTable, dataTable,
resolvesOnBackend,
+ handle.getScanOptions());
+ }
+
+ /**
+ * Re-apply the relation-scoped scan params to a rebuilt system-table
wrapper.
+ *
+ * <p>{@link #resolveScanTable} applies {@code @incr} / {@code @options}
to the wrapper the
+ * handle carries, and every Paimon system table delegates {@code
copy(...)} to the data table it
+ * wraps. The wrapper rebuilt above is a different object, so the same
copy has to be redone on
+ * it, otherwise the BE would materialize its splits against the unpinned
latest state. The
+ * branches mirror {@link #resolveScanTable}'s exactly — one chokepoint's
worth of logic applied
+ * to two different objects.
+ *
+ * <p>This runs last on purpose: an explicit relation option outranks
anything this class pins on
+ * the rebuilt table, and {@code copy(...)} lets the option win. An
incremental relation outranks
+ * {@link #pinCatalogSnapshot} the same way but cannot inherit its bound,
so it is bound to the
+ * catalog's snapshot separately by {@link
PaimonIncrementalScanParams#bindRangeToCatalog}.
+ *
+ * <p>Known gap: the {@code @options} branch reaches tables outside
+ * {@link PaimonScanParams#resolvesSnapshotOnBackend}, {@code $ro} among
them, and on a
+ * {@code scan.fallback-branch} table the {@code copy(...)} here is what
triggers
+ * {@code rewriteFallbackOptions} - on the already catalog-less pair, so
the fallback branch's
+ * {@code scan.snapshot-id} is derived from its snapshot directory rather
than from the catalog
+ * pointer, the same gap {@link #pinCatalogSnapshot} documents. It lands
harder here: unlike the
+ * pin this is {@code copy(...)}, not {@code copyWithoutTimeTravel(...)},
so it also time-travels
+ * the fallback branch's schema, and {@code $ro} is a data table rather
than read-only metadata.
+ */
+ private Table reapplyScanParams(Table rebuiltSysTable, FileStoreTable
dataTable,
+ boolean resolvesOnBackend, Map<String, String> scanOptions) {
+ if (scanOptions == null || scanOptions.isEmpty()) {
+ return rebuiltSysTable;
+ }
+ if (PaimonScanParams.isOptionsPin(scanOptions)) {
+ return PaimonScanParams.applyOptions(rebuiltSysTable, scanOptions);
+ }
+ Map<String, String> params = resolvesOnBackend
+ ? PaimonIncrementalScanParams.bindRangeToCatalog(scanOptions,
dataTable)
+ : scanOptions;
+ return
rebuiltSysTable.copy(PaimonIncrementalScanParams.applyResetsIfIncremental(params));
+ }
+
+ /**
+ * {@code $files} plans only partition-level splits on the FE and re-plans
the base table on the
+ * BE through {@code DataTableScan#plan()} ({@code
FilesTable.FilesRead#createReader}). That
+ * deferred plan normally authorizes itself through the catalog loader
+ * ({@code CatalogEnvironment#tableQueryAuth} -> {@code
Catalog#authTableQuery}); once the
+ * loader is dropped it silently allows everything. So authorize here,
while the loader is still
+ * around. Paimon discards the predicates the call returns (row level
access control is a TODO in
+ * {@code AbstractDataTableScan#authQuery}), so running it on the FE loses
nothing.
+ *
+ * <p>Only {@code $files} may do this: {@code auth(null)} means "every
column" to
+ * {@code Catalog#authTableQuery}, and the system tables that keep
planning on the FE
+ * ({@code $ro}, {@code $row_tracking}, {@code $audit_log}, {@code
$binlog}) already authorize
+ * themselves through {@code DataTableBatchScan} with the slot projection
the query really reads.
+ * Authorizing those again for every column would reject a user allowed to
read only some of the
+ * base columns. {@code $partitions} never reaches {@code plan()} on
either side
+ * ({@code listPartitionEntries} does not authorize), so it has no
authorization to transfer.
+ *
+ * <p>A {@code scan.fallback-branch} table has to be authorized branch by
branch.
+ * {@code FallbackReadFileStoreTable#newScan} builds a {@code
FallbackReadScan} over both
+ * branches' own scans, so each authorizes itself, and {@code
FileStoreTableFactory#create} gives
+ * the fallback branch a {@link CatalogEnvironment} of its own carrying a
branch-qualified
+ * {@code Identifier}. Since the pair delegates {@code
catalogEnvironment()} to its main branch,
+ * authorizing the pair would check the main branch alone and never the
fallback one - and once
+ * its loader is dropped that missing check turns into a permanent allow,
letting a user denied on
+ * the fallback branch read the fallback rows of {@code $files}.
+ */
+ // Package-private for direct unit testing (PaimonBackendBoundTableTest).
+ static void authorizeDeferredScan(FileStoreTable dataTable) {
+ FileStoreTable undecorated =
PaimonTableDecorators.unwrapToFallbackOrBase(dataTable);
+ if (undecorated instanceof FallbackReadFileStoreTable) {
+ FallbackReadFileStoreTable fallbackReadTable =
(FallbackReadFileStoreTable) undecorated;
+ authorizeBranch(fallbackReadTable.wrapped());
+ authorizeBranch(fallbackReadTable.fallback());
Review Comment:
[P1] Preserve conditional fallback authorization
`FallbackReadScan.plan()` always plans the main scan, but invokes
`fallbackScan.plan()` only when `listPartitions()` finds partitions not
completed by main; that listing itself does not authorize. This unconditional
call therefore rejects a `$files` query for a user denied on the fallback
branch even when every selected partition is satisfied by main and Paimon would
never invoke fallback's authorized `plan()` or return fallback files. This is
the inverse of the earlier missing-auth case: please transfer the fallback
authorization with the same branch-use condition (while retaining the check
when fallback contributes), and cover a
main-complete/main-allowed/fallback-denied case.
--
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]
