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]

Reply via email to