drexler-sky commented on code in PR #16891:
URL: https://github.com/apache/iceberg/pull/16891#discussion_r3479407551
##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/TestSparkCatalogOperations.java:
##########
@@ -122,6 +128,81 @@ public void testAlterTable() throws NoSuchTableException {
.containsEntry(propsKey, propsValue);
}
+ @TestTemplate
+ public void testTableTypeIsExternal() throws NoSuchTableException {
+ BaseCatalog catalog = (BaseCatalog)
spark.sessionState().catalogManager().catalog(catalogName);
+ Identifier identifier = Identifier.of(tableIdent.namespace().levels(),
tableIdent.name());
+
+ Table table = catalog.loadTable(identifier);
+
+ assertThat(table.properties())
+ .as("Iceberg tables should report an EXTERNAL table type")
+ .containsEntry(TableCatalog.PROP_TABLE_TYPE,
TableSummary.EXTERNAL_TABLE_TYPE);
+ }
+
+ @TestTemplate
+ public void testListTableSummaries() throws NoSuchNamespaceException,
NoSuchTableException {
+ BaseCatalog catalog = (BaseCatalog)
spark.sessionState().catalogManager().catalog(catalogName);
+ Identifier identifier = Identifier.of(tableIdent.namespace().levels(),
tableIdent.name());
+
+ TableSummary[] summaries =
catalog.listTableSummaries(tableIdent.namespace().levels());
+
+ assertThat(Arrays.stream(summaries))
+ .as("Listed table summary should report the table as EXTERNAL")
+ .anySatisfy(
+ summary -> {
+ assertThat(summary.identifier()).isEqualTo(identifier);
+
assertThat(summary.tableType()).isEqualTo(TableSummary.EXTERNAL_TABLE_TYPE);
+ });
+ }
+
+ @TestTemplate
+ public void testListTableSummariesIncludesViews()
+ throws NoSuchNamespaceException, NoSuchTableException {
+ BaseCatalog catalog = (BaseCatalog)
spark.sessionState().catalogManager().catalog(catalogName);
+ // Only SparkCatalog overrides listTableSummaries, and views require a
view-capable catalog
+ // (e.g. Hive). Hadoop catalogs and the session catalog are skipped.
+ assumeThat(catalog).isInstanceOf(SparkCatalog.class);
+ assumeThat(validationCatalog)
+ .as("Requires a catalog that supports views")
+ .isInstanceOf(ViewCatalog.class);
+
+ // Create the view directly through the Iceberg ViewCatalog API. SQL
CREATE VIEW against a v2
+ // catalog requires the Iceberg Spark extensions, which this module's
tests do not load.
+ ViewCatalog viewCatalog = (ViewCatalog) validationCatalog;
+ TableIdentifier viewIdent = TableIdentifier.of(tableIdent.namespace(),
"summary_view");
+ viewCatalog
+ .buildView(viewIdent)
+ .withSchema(new Schema(Types.NestedField.optional(1, "id",
Types.LongType.get())))
+ .withDefaultNamespace(tableIdent.namespace())
+ .withQuery("spark", "SELECT id FROM " + tableIdent.name())
+ .create();
+
+ try {
+ String[] namespace = tableIdent.namespace().levels();
+ Identifier tableIdentifier = Identifier.of(namespace, tableIdent.name());
+ Identifier viewIdentifier = Identifier.of(namespace, "summary_view");
+
+ TableSummary[] summaries = catalog.listTableSummaries(namespace);
+
+ assertThat(summaries)
+ .as("Base table should be reported exactly once as EXTERNAL")
+ .filteredOn(summary -> summary.identifier().equals(tableIdentifier))
+ .singleElement()
+ .extracting(TableSummary::tableType)
+ .isEqualTo(TableSummary.EXTERNAL_TABLE_TYPE);
+
+ assertThat(summaries)
+ .as("View should be reported exactly once as VIEW, never as a table")
Review Comment:
Fixed. Thanks!
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -368,6 +369,31 @@ public Identifier[] listTables(String[] namespace) {
.toArray(Identifier[]::new);
}
+ @Override
+ public TableSummary[] listTableSummaries(String[] namespace) {
+ // Build summaries directly from the catalog listings to avoid loading
every table, which the
+ // default TableCatalog.listTableSummaries implementation would do.
Iceberg tables are always
+ // reported as EXTERNAL (see SparkTable#properties), and views are
reported as VIEW.
Review Comment:
Done. Thanks!
##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/TestSparkCatalogOperations.java:
##########
@@ -122,6 +128,81 @@ public void testAlterTable() throws NoSuchTableException {
.containsEntry(propsKey, propsValue);
}
+ @TestTemplate
+ public void testTableTypeIsExternal() throws NoSuchTableException {
+ BaseCatalog catalog = (BaseCatalog)
spark.sessionState().catalogManager().catalog(catalogName);
+ Identifier identifier = Identifier.of(tableIdent.namespace().levels(),
tableIdent.name());
+
+ Table table = catalog.loadTable(identifier);
+
+ assertThat(table.properties())
+ .as("Iceberg tables should report an EXTERNAL table type")
+ .containsEntry(TableCatalog.PROP_TABLE_TYPE,
TableSummary.EXTERNAL_TABLE_TYPE);
+ }
+
+ @TestTemplate
+ public void testListTableSummaries() throws NoSuchNamespaceException,
NoSuchTableException {
Review Comment:
Folded into one test.
--
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]
