github-actions[bot] commented on code in PR #65867:
URL: https://github.com/apache/doris/pull/65867#discussion_r3649667208
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -190,6 +193,58 @@ protected void doInitialize() throws UserException {
}
}
+ /**
+ * 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
org.apache.paimon.table.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(...)}, 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.</li>
+ * </ul>
+ */
+ private Table getPaimonTableForBackend() {
+ Table paimonTable = source.getPaimonTable();
+ if (paimonTable instanceof FileStoreTable) {
+ return dropCatalogLoader((FileStoreTable) paimonTable);
+ }
+ if (!(source.getExternalTable() instanceof PaimonSysExternalTable)) {
+ return paimonTable;
+ }
+ PaimonSysExternalTable sysTable = (PaimonSysExternalTable)
source.getExternalTable();
+ // System tables ignore snapshot semantics, so the empty snapshot
resolves the base table.
+ Table dataTable =
sysTable.getSourceTable().getPaimonTable(Optional.empty());
Review Comment:
[P1] Build this wrapper from the same system-table generation
`source.getPaimonTable()` is the system wrapper loaded directly by
`PaimonSysExternalTable`, but this line fetches its base through a separate
`PaimonExternalMetaCache` entry. Those objects refresh independently, so after
an external schema change the FE can plan a v2 `$audit_log`/`$binlog` wrapper
while the serialized BE wrapper is rebuilt over cached v1. Both tables are
forced through JNI; `getProjected()` then rejects v2 fields absent from v1 (or
reads with stale same-name types). Please construct the catalog-less wrapper
from the exact base table used by the FE wrapper, and cover a
schema-evolution/cache-skew case.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -190,6 +193,58 @@ protected void doInitialize() throws UserException {
}
}
+ /**
+ * 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
org.apache.paimon.table.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(...)}, 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.</li>
+ * </ul>
+ */
+ private Table getPaimonTableForBackend() {
+ Table paimonTable = source.getPaimonTable();
+ if (paimonTable instanceof FileStoreTable) {
+ return dropCatalogLoader((FileStoreTable) paimonTable);
+ }
+ if (!(source.getExternalTable() instanceof PaimonSysExternalTable)) {
+ return paimonTable;
+ }
+ PaimonSysExternalTable sysTable = (PaimonSysExternalTable)
source.getExternalTable();
+ // System tables ignore snapshot semantics, so the empty snapshot
resolves the base table.
+ Table dataTable =
sysTable.getSourceTable().getPaimonTable(Optional.empty());
+ if (!(dataTable instanceof FileStoreTable)) {
+ return paimonTable;
+ }
+ Table catalogLessSysTable = SystemTableLoader.load(
+ sysTable.getSysTableType(), dropCatalogLoader((FileStoreTable)
dataTable));
+ return catalogLessSysTable == null ? paimonTable : catalogLessSysTable;
+ }
+
+ /**
+ * Return an equivalent {@link FileStoreTable} without the catalog loader,
so the BE never
+ * deserializes a {@code HiveCatalogLoader} (and snapshot loading uses the
filesystem instead of
+ * the catalog's metastore). fileIO / location / schema (and the schema's
options) are preserved.
+ */
+ private static FileStoreTable dropCatalogLoader(FileStoreTable dataTable) {
+ if (dataTable.catalogEnvironment().catalogLoader() == null) {
+ return dataTable;
+ }
+ return FileStoreTableFactory.create(
Review Comment:
[P1] Preserve REST catalog semantics for BE-side `$files` planning
`$files` only creates outer partition splits on FE;
`FilesRead.createReader()` later calls `storeTable.newScan().plan()` on BE.
Replacing the whole `CatalogEnvironment` here therefore drops two contracts
that this deferred scan still needs:
- With `query.auth-enabled`, `tableQueryAuth()` normally calls REST
`authTableQuery`; a null loader makes it a no-op, so a denial becomes a
successful metadata read.
- For version-managed REST catalogs, `SnapshotManager` normally uses the
catalog's committed-snapshot pointer. With an empty environment it scans
filesystem latest. Paimon's commit path publishes the snapshot file before
advancing that pointer, so BE can return snapshot N files while FE/catalog
remains at N-1, and readers can mix generations.
Please authorize and pin the catalog-visible snapshot on FE before
serialization (or preserve equivalent loader-free state), and add denied-auth
plus publication-window `$files` tests.
--
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]