harshitaajoshi commented on code in PR #17526:
URL: https://github.com/apache/iceberg/pull/17526#discussion_r3723945397
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java:
##########
@@ -307,6 +308,68 @@ public void testInitSchemaV0() {
.hasMessage(JdbcCatalog.VIEW_WARNING_LOG_MESSAGE);
}
+ @Test
+ void catalogTablesAreCreatedWhenAnotherTableMatchesTheirNamePattern() throws
Exception {
+ // as this test uses different connections, we can't use memory database
(as it's per
+ // connection), but a file database instead
+ java.nio.file.Path dbFile = Files.createTempFile("icebergSimilarTable",
"db");
+ String jdbcUrl = "jdbc:sqlite:" + dbFile.toAbsolutePath();
+
+ // the underscore in iceberg_tables is a wildcard when used as a JDBC
metadata pattern, so
+ // this unrelated table is reported as the catalog table unless the
pattern is escaped
+ executeUpdate(jdbcUrl, "CREATE TABLE iceberg1tables (col VARCHAR(255))");
+
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put(CatalogProperties.WAREHOUSE_LOCATION,
this.tableDir.toAbsolutePath().toString());
+ properties.put(CatalogProperties.URI, jdbcUrl);
+
+ try (JdbcCatalog jdbcCatalog = new JdbcCatalog()) {
+ jdbcCatalog.setConf(conf);
+ jdbcCatalog.initialize("similar_table_catalog", properties);
+
+ assertThat(catalogTablesExist(jdbcUrl)).isTrue();
+
+ TableIdentifier tableIdent = TableIdentifier.of(Namespace.of("ns1"),
"tbl");
+ jdbcCatalog.buildTable(tableIdent, SCHEMA).create();
+ assertThat(jdbcCatalog.loadTable(tableIdent).schema().asStruct())
+ .isEqualTo(SCHEMA.asStruct());
+ }
+ }
+
+ @Test
+ void schemaVersionIgnoresColumnsOfTablesMatchingTheCatalogTableNamePattern()
throws Exception {
+ // as this test uses different connections, we can't use memory database
(as it's per
+ // connection), but a file database instead
+ java.nio.file.Path dbFile = Files.createTempFile("icebergSimilarColumn",
"db");
+ String jdbcUrl = "jdbc:sqlite:" + dbFile.toAbsolutePath();
+
+ // create the catalog tables up front so that only schema version
detection is exercised
+ executeUpdate(jdbcUrl, JdbcUtil.V0_CREATE_CATALOG_SQL);
+ executeUpdate(jdbcUrl, JdbcUtil.CREATE_NAMESPACE_PROPERTIES_TABLE_SQL);
+ // this table and its column match iceberg_tables and iceberg_type through
the underscore
+ // wildcard, so an unescaped lookup reports view support that the catalog
table lacks
+ executeUpdate(jdbcUrl, "CREATE TABLE iceberg1tables (iceberg1type
VARCHAR(5))");
+
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put(CatalogProperties.WAREHOUSE_LOCATION,
this.tableDir.toAbsolutePath().toString());
+ properties.put(CatalogProperties.URI, jdbcUrl);
+
+ try (JdbcCatalog jdbcCatalog = new JdbcCatalog()) {
+ jdbcCatalog.setConf(conf);
+ jdbcCatalog.initialize("similar_column_catalog", properties);
+
+ // committing as V1 would write iceberg_type, which this V0 catalog
table does not have
+ TableIdentifier tableIdent = TableIdentifier.of(Namespace.of("ns1"),
"tbl");
+ jdbcCatalog.buildTable(tableIdent, SCHEMA).create();
+ assertThat(jdbcCatalog.loadTable(tableIdent).schema().asStruct())
+ .isEqualTo(SCHEMA.asStruct());
+
+ assertThatThrownBy(() -> jdbcCatalog.listViews(Namespace.of("ns1")))
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessage(JdbcCatalog.VIEW_WARNING_LOG_MESSAGE);
+ }
+ }
+
Review Comment:
Agreed that this is the gap, and I tried to write it. Neither test driver
can express it:
- SQLite does not surface `ATTACH DATABASE` databases as JDBC catalogs.
`getCatalogs()` returns nothing and `getTables` only sees the main database, so
an attached `iceberg_tables` is invisible to the metadata API.
- Derby reports no catalog at all.
So a genuine two catalog test needs MySQL or PostgreSQL, and `iceberg-core`
has no Testcontainers setup. The catalog resolution itself is unit tested in
`TestJdbcUtil`, and the escaping half has end to end coverage in
`TestJdbcCatalog`.
If the project is open to it I am happy to add a Testcontainers based MySQL
test, either here or as a follow-up. That felt like a bigger call than this fix
should make on its own, so I would rather a committer weigh in.
--
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]
