github-actions[bot] commented on code in PR #65867:
URL: https://github.com/apache/doris/pull/65867#discussion_r3650529059
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -190,6 +194,99 @@ 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. The base table is the one the
FE-side wrapper was
+ * built over, and what the catalog would have done on the BE is
done here instead:
+ * see {@link #authorizeDeferredScan} and {@link
#pinCatalogSnapshot}.</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();
+ // 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 = sysTable.getSysBaseTable();
+ if (dataTable == null) {
+ return paimonTable;
+ }
+ authorizeDeferredScan(dataTable);
+ Table catalogLessSysTable = SystemTableLoader.load(
+ sysTable.getSysTableType(),
pinCatalogSnapshot(dropCatalogLoader(dataTable), dataTable));
+ return catalogLessSysTable == null ? paimonTable : catalogLessSysTable;
+ }
+
+ /**
+ * {@code $files} and {@code $partitions} only plan partition-level splits
on the FE and re-plan
+ * the base table on the BE ({@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.
+ */
+ private static void authorizeDeferredScan(FileStoreTable dataTable) {
+ CoreOptions options = dataTable.coreOptions();
+ if (options.queryAuthEnabled()) {
+ dataTable.catalogEnvironment().tableQueryAuth(options).auth(null);
+ }
+ }
+
+ /**
+ * For catalogs that manage versions themselves (Paimon REST / DLF REST)
the committed snapshot
+ * is the one the catalog points at, not the newest file in the snapshot
directory: Paimon
+ * publishes the snapshot file before the pointer moves, and a rollback
leaves newer files
+ * behind. Without the catalog loader {@code SnapshotManager} falls back
to listing that
+ * directory, so the BE could plan on a snapshot the catalog has not
published while the FE
+ * planned on the previous one. Pin the catalog-visible snapshot instead.
+ */
+ private static FileStoreTable pinCatalogSnapshot(FileStoreTable
catalogLessTable, FileStoreTable dataTable) {
+ if (!dataTable.catalogEnvironment().supportsVersionManagement()) {
+ return catalogLessTable;
+ }
+ Long snapshotId = dataTable.snapshotManager().latestSnapshotId();
+ if (snapshotId == null) {
+ return catalogLessTable;
+ }
+ // Without time travel: pin which snapshot the BE plans on, leave
schema resolution alone.
+ return catalogLessTable.copyWithoutTimeTravel(
Review Comment:
[P1] Keep the pin from changing the BE schema
`copyWithoutTimeTravel` preserves the FE schema only at this point. After
deserialization, `PaimonJniScanner.initTable()` unconditionally calls
`table.copy(...)`; Paimon's `$audit_log`/`$binlog`/`$ro`/`$row_tracking`
wrappers delegate that to ordinary `FileStoreTable.copy`, which sees the
retained `scan.snapshot-id` and time-travels the schema. If the catalog schema
S2 adds `c2` after the latest data snapshot N (schema S1), FE plans `c2` from
S2 but BE rewinds to S1 and fails with `RequiredField c2 not found in schema`.
Please carry the bound in a form the BE option-refresh copy cannot reinterpret,
and cover an encode/decode plus BE-copy schema-evolution case.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -190,6 +194,99 @@ 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. The base table is the one the
FE-side wrapper was
+ * built over, and what the catalog would have done on the BE is
done here instead:
+ * see {@link #authorizeDeferredScan} and {@link
#pinCatalogSnapshot}.</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();
+ // 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 = sysTable.getSysBaseTable();
+ if (dataTable == null) {
+ return paimonTable;
+ }
+ authorizeDeferredScan(dataTable);
+ Table catalogLessSysTable = SystemTableLoader.load(
+ sysTable.getSysTableType(),
pinCatalogSnapshot(dropCatalogLoader(dataTable), dataTable));
+ return catalogLessSysTable == null ? paimonTable : catalogLessSysTable;
+ }
+
+ /**
+ * {@code $files} and {@code $partitions} only plan partition-level splits
on the FE and re-plan
+ * the base table on the BE ({@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.
+ */
+ private static void authorizeDeferredScan(FileStoreTable dataTable) {
+ CoreOptions options = dataTable.coreOptions();
+ if (options.queryAuthEnabled()) {
+ dataTable.catalogEnvironment().tableQueryAuth(options).auth(null);
Review Comment:
[P1] Preserve projected authorization for data system tables
This helper runs for every Paimon system table, but `null` means “select
all” in `Catalog.authTableQuery`. `$ro` and `$row_tracking` already plan on FE
through `DataTableBatchScan`; Doris applies the actual slot projection first,
so their normal call is `auth([selected fields])`. A user allowed to read `c1`
but not every base column can therefore run `SELECT c1 FROM tbl$ro` through the
normal authorization, yet this earlier `auth(null)` rejects it. Please transfer
authorization only for the deferred paths that lose it (such as `$files`), or
preserve the exact projected field list, and add an argument-sensitive test.
##########
fe/be-java-extensions/paimon-scanner/pom.xml:
##########
@@ -61,6 +61,31 @@ under the License.
<artifactId>paimon-format</artifactId>
</dependency>
+ <!--
+ Paimon OSS/S3 FileIO plugins. These register
org.apache.paimon.fs.FileIOLoader
+ service providers (paimon-s3 -> S3Loader, paimon-jindo ->
JindoLoader) used to read
+ object-store-backed Paimon tables (e.g. DLF REST catalogs over OSS).
They MUST be
+ bundled here, alongside the FileIOLoader interface that lives in
paimon-common: when a
+ BE-side read resolves the latest snapshot from the filesystem,
RESTTokenFileIO.fileIO()
+ calls FileIO.get() -> ServiceLoader.load(FileIOLoader.class,
FileIOLoader's classloader),
+ which eagerly instantiates every registered provider. A provider and
the FileIOLoader
+ interface it implements must be reachable from the same classloader.
These plugins used
+ to sit on the shared preload-extensions (JVM app) classpath, which
does NOT carry
+ paimon-common's FileIOLoader; since JniScannerClassLoader is
parent-first, the app-loaded
+ S3Loader/JindoLoader could not resolve the child-only interface ->
NoClassDefFoundError:
+ FileIOLoader. Co-locating them here keeps the whole FileIO SPI in
one classloader; the
+ assembly's metaInf-services handler merges their service files with
paimon-common's
+ (local/hadoop) into one complete provider list.
+ -->
+ <dependency>
+ <groupId>org.apache.paimon</groupId>
+ <artifactId>paimon-s3</artifactId>
Review Comment:
[P1] Add a regression test for the classloader boundary being fixed
The added FE tests use `LocalFileIO` with all Paimon artifacts in one Maven
test classloader, so they cannot reproduce this PR's
`JniScannerClassLoader`/preload failure. The dedicated “Build Extensions”
workflow also only detects paths; it does not build or inspect these
assemblies. A missing merged service entry or provider left on the parent
classpath would still pass every current test and fail at runtime. Please add a
packaging-level test that loads the built preload jar as parent and
paimon-scanner jar as child, verifies the provider ownership and merged
`FileIOLoader` services, and exercises `FileIO.get` for S3/Jindo.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -190,6 +194,99 @@ 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. The base table is the one the
FE-side wrapper was
+ * built over, and what the catalog would have done on the BE is
done here instead:
+ * see {@link #authorizeDeferredScan} and {@link
#pinCatalogSnapshot}.</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();
+ // 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 = sysTable.getSysBaseTable();
+ if (dataTable == null) {
+ return paimonTable;
+ }
+ authorizeDeferredScan(dataTable);
+ Table catalogLessSysTable = SystemTableLoader.load(
+ sysTable.getSysTableType(),
pinCatalogSnapshot(dropCatalogLoader(dataTable), dataTable));
+ return catalogLessSysTable == null ? paimonTable : catalogLessSysTable;
+ }
+
+ /**
+ * {@code $files} and {@code $partitions} only plan partition-level splits
on the FE and re-plan
+ * the base table on the BE ({@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.
+ */
+ private static void authorizeDeferredScan(FileStoreTable dataTable) {
+ CoreOptions options = dataTable.coreOptions();
+ if (options.queryAuthEnabled()) {
+ dataTable.catalogEnvironment().tableQueryAuth(options).auth(null);
+ }
+ }
+
+ /**
+ * For catalogs that manage versions themselves (Paimon REST / DLF REST)
the committed snapshot
+ * is the one the catalog points at, not the newest file in the snapshot
directory: Paimon
+ * publishes the snapshot file before the pointer moves, and a rollback
leaves newer files
+ * behind. Without the catalog loader {@code SnapshotManager} falls back
to listing that
+ * directory, so the BE could plan on a snapshot the catalog has not
published while the FE
+ * planned on the previous one. Pin the catalog-visible snapshot instead.
+ */
+ private static FileStoreTable pinCatalogSnapshot(FileStoreTable
catalogLessTable, FileStoreTable dataTable) {
Review Comment:
[P1] Preserve the catalog bound for metadata readers that ignore
scan.snapshot-id
This option is not a universal system-table pin. In Paimon 1.3.1,
`$snapshots` reads `SnapshotManager.snapshotsWithinRange(...)` and `$buckets`
calls `newSnapshotReader().bucketEntries()`; neither consumes
`scan.snapshot-id`. Once the loader is removed, both can resolve filesystem N+1
while the REST/DLF catalog still exposes N (or after a rollback). When
`latestSnapshotId()` is null, the branch below returns an entirely unpinned
table and can expose an orphan first snapshot as well. Please preserve an
explicit nullable catalog bound that these readers consume (or materialize them
on FE), with publication-window, rollback, and catalog-visible-empty 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]
