sdaberdaku commented on code in PR #17862:
URL: https://github.com/apache/iceberg/pull/17862#discussion_r3883826056
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/DeleteOrphanFilesSparkAction.java:
##########
@@ -222,6 +223,24 @@ public DeleteOrphanFilesSparkAction
usePrefixListing(boolean newUsePrefixListing
return this;
}
+ /**
+ * Configures the listing to use the Hadoop configuration of the given
catalog.
+ *
+ * <p>Listing the table location goes through the Hadoop {@link
org.apache.hadoop.fs.FileSystem}
+ * API, which is configured from the session and so reaches storage as a
different principal than
+ * the catalog does. Setting the catalog makes the listing follow the
catalog's own configuration
+ * instead.
+ *
+ * @param newCatalogName the name of the catalog that holds the table
+ * @return this for method chaining
+ */
+ public DeleteOrphanFilesSparkAction catalogName(String newCatalogName) {
Review Comment:
Thinking about this more, there's a way to avoid the knob entirely. Every
catalog-loaded table is named `<catalog>.<namespace>.<table>` by
`CatalogUtil.fullTableName`, and the catalog part is the Spark catalog name
that `SparkCatalog.buildIcebergCatalog` passed in (`spark_catalog` for
`SparkSessionCatalog`). So the action can resolve the catalog itself: take the
first segment of `table.name()`, check it's a registered catalog via
`CatalogManager.isCatalogRegistered`, and if so build the Hadoop configuration
with `SparkUtil.hadoopConfCatalogOverrides`. Anything else, including
path-based tables, falls back to the session configuration, which is today's
behaviour.
That drops `catalogName(...)` and the procedure change altogether;
`RemoveOrphanFilesProcedure` stays as it is on main, and direct users of
`SparkActions.deleteOrphanFiles(table)` get the same fix. It's the same
first-segment convention `Spark3Util.catalogAndIdentifier` relies on, but
without the current-catalog fallback, since applying another catalog's
overrides to a foreign table would be wrong.
The only gap I see is a custom `SparkCatalog` subclass that gives its
Iceberg catalog a different name; that just falls back to session behaviour.
I'll push this shape unless you'd rather not rely on the table name.
--
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]
