suxiaogang223 commented on code in PR #62988:
URL: https://github.com/apache/doris/pull/62988#discussion_r4385218199
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogFactory.java:
##########
@@ -533,10 +534,21 @@ public static void appendRestProperties(Map<String,
String> opts, IcebergRestMet
// Timeouts: legacy fields default non-blank, so they are effectively
always emitted.
opts.put(REST_CONNECTION_TIMEOUT_MS_KEY,
rest.getConnectionTimeoutMs());
opts.put(REST_SOCKET_TIMEOUT_MS_KEY, rest.getSocketTimeoutMs());
+ putIfNotBlank(opts, CatalogProperties.FILE_IO_IMPL, rest.getIoImpl());
+ putIfNotBlank(opts, "header.x-goog-user-project",
rest.getGoogleUserProject());
+ putIfNotBlank(opts, "gcs.oauth2.token", rest.getGcsOauth2Token());
Review Comment:
[P1] Wire Google storage credentials through to the BE data reader
These properties configure the FE's Iceberg FileIO, but ordinary Iceberg
data files are read by the BE's native reader rather than GCSFileIO. With the
ADC-only configuration shown in the PR description and a private GCS bucket,
REST/manifest access on the FE can succeed while a query that actually reads a
data column still fails: the existing GCS storage implementation uses the
S3/XML API with HMAC credentials, and this change supplies neither usable BE
credentials nor an OAuth2-capable BE read path. Credential vending does not
bridge the gap either: IcebergScanPlanProvider passes the FileIO credentials
through DefaultConnectorContext.vendStorageCredentials(), whose
CredentialUtils.filterCloudStorageProperties() accepts `gs.*` but drops
`gcs.oauth2.token`.
Could we complete the BE storage authentication path, or explicitly require
separate HMAC credentials and update the example and supported scope
accordingly? Please also cover a query that reads an actual data column from a
private bucket; COUNT(*) can be answered from metadata and would miss this
failure.
##########
fe/fe-connector/fe-connector-iceberg/pom.xml:
##########
@@ -112,6 +112,20 @@ under the License.
<version>${iceberg.version}</version>
</dependency>
+ <!-- GoogleAuthManager and GCSFileIO are loaded by name inside the
connector plugin. -->
+ <dependency>
+ <groupId>org.apache.iceberg</groupId>
+ <artifactId>iceberg-gcp</artifactId>
Review Comment:
[P2] Include the GCS FileIO runtime in the BE metadata scanner plugin
Adding these dependencies only to the FE connector leaves the BE's isolated
`iceberg-metadata-scanner` plugin without `iceberg-gcp` and its Google runtime
dependencies. For a non-empty GCS-backed table, a `$files` scan produces
Iceberg BaseFilesTable.ManifestReadTask objects that retain `table.io()`
(GCSFileIO). IcebergScanPlanProvider serializes those tasks into
`serialized_split`, and IcebergSysTableJniScanner deserializes them on the BE.
That deserialization cannot resolve `org.apache.iceberg.gcp.gcs.GCSFileIO` with
the scanner's current dependency closure, so the metadata query fails before it
can read the manifest. IcebergSerializationCompat also wraps the missing-class
exception as an Iceberg version mismatch, which obscures the cause.
Please add the required runtime dependencies to the BE scanner plugin as
well and cover a `$files` query on a non-empty GCS table. The new
googleRuntimeClassesAreAvailable() test checks only the FE connector
classloader and cannot detect this packaging gap.
--
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]
