wombatu-kun commented on code in PR #16891:
URL: https://github.com/apache/iceberg/pull/16891#discussion_r3463900404
##########
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:
The `never as a table` half of this assertion passes trivially: none of this
test's three catalog configs set `list-all-tables=true`, so
`HiveCatalog.listTables` filters to Iceberg tables only
(HiveCatalog.java:173-182) and never returns the view. The de-dup guard it is
meant to cover, `!viewIdents.contains(ident)` at `SparkCatalog.java:385`, is
therefore never exercised - deleting that guard would not fail any test here.
To actually cover it, add a SparkCatalog-over-HiveCatalog config with
`list-all-tables=true` so `listTables` returns the view, then assert the view
is reported once as VIEW and not also as EXTERNAL.
--
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]
