manuzhang commented on code in PR #15240:
URL: https://github.com/apache/iceberg/pull/15240#discussion_r2869139331
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/IcebergSource.java:
##########
@@ -99,96 +95,74 @@ public boolean supportsExternalMetadata() {
@Override
public Table getTable(StructType schema, Transform[] partitioning,
Map<String, String> options) {
- Spark3Util.CatalogAndIdentifier catalogIdentifier =
- catalogAndIdentifier(new CaseInsensitiveStringMap(options));
- CatalogPlugin catalog = catalogIdentifier.catalog();
- Identifier ident = catalogIdentifier.identifier();
+ return loadTable(new CaseInsensitiveStringMap(options));
+ }
+ private Table loadTable(CaseInsensitiveStringMap options) {
+ CatalogAndIdentifier catalogAndIdent = catalogAndIdentifier(options);
+ TableCatalog catalog = catalogAndIdent.tableCatalog();
+ Identifier ident = catalogAndIdent.identifier();
try {
- if (catalog instanceof TableCatalog) {
- return ((TableCatalog) catalog).loadTable(ident);
- }
+ return catalog.loadTable(ident);
} catch (NoSuchTableException e) {
- // throwing an iceberg NoSuchTableException because the Spark one is
typed and can't be thrown
- // from this interface
+ // TableProvider doesn't permit typed exception while loading tables,
+ // so throw Iceberg NoSuchTableException because the Spark one is typed
throw new org.apache.iceberg.exceptions.NoSuchTableException(
- e, "Cannot find table for %s.", ident);
+ e,
+ "Cannot find table %s in catalog %s (%s)",
+ ident,
+ catalog.name(),
+ catalog.getClass().getName());
}
-
- // throwing an iceberg NoSuchTableException because the Spark one is typed
and can't be thrown
- // from this interface
- throw new org.apache.iceberg.exceptions.NoSuchTableException(
- "Cannot find table for %s.", ident);
}
- private Spark3Util.CatalogAndIdentifier
catalogAndIdentifier(CaseInsensitiveStringMap options) {
+ private CatalogAndIdentifier catalogAndIdentifier(CaseInsensitiveStringMap
options) {
Preconditions.checkArgument(
options.containsKey(SparkReadOptions.PATH), "Cannot open table: path
is not set");
+ Spark3Util.validateNoLegacyTimeTravel(options);
+
SparkSession spark = SparkSession.active();
+ CatalogManager catalogManager = spark.sessionState().catalogManager();
Review Comment:
Why is this moved to far from where it's firstly used?
--
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]
