amogh-jahagirdar commented on code in PR #10111:
URL: https://github.com/apache/iceberg/pull/10111#discussion_r1565994045
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java:
##########
@@ -1024,7 +1028,49 @@ public void report(MetricsReport report) {
}
}
+ private String createMetadataLocationViaJdbcCatalog(TableIdentifier
identifier)
+ throws SQLException {
+ // temporary connection just to actually create a concrete metadata
location
+ String jdbcUrl = null;
+ try {
+ java.nio.file.Path dbFile = Files.createTempFile("temp", "metadata");
+ jdbcUrl = "jdbc:sqlite:" + dbFile.toAbsolutePath();
+ } catch (IOException e) {
+ throw new SQLException("Error while creating temp data", e);
+ }
+
+ Map<String, String> properties = Maps.newHashMap();
+
+ properties.put(CatalogProperties.URI, jdbcUrl);
+
+ warehouseLocation = this.tableDir.toAbsolutePath().toString();
+ properties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouseLocation);
+ properties.put("type", "jdbc");
+
+ JdbcCatalog jdbcCatalog =
+ (JdbcCatalog) CatalogUtil.buildIcebergCatalog("TEMP", properties,
conf);
+ jdbcCatalog.buildTable(identifier, SCHEMA).create();
+
+ SQLiteDataSource dataSource = new SQLiteDataSource();
+ dataSource.setUrl(jdbcUrl);
+
+ try (Connection connection = dataSource.getConnection()) {
+ ResultSet result =
+ connection
+ .prepareStatement("SELECT * FROM " +
JdbcUtil.CATALOG_TABLE_VIEW_NAME)
+ .executeQuery();
+ result.next();
+ return result.getString(JdbcTableOperations.METADATA_LOCATION_PROP);
+ }
+ }
+
private void initLegacySchema(String jdbcUrl) throws SQLException {
+ TableIdentifier table1 = TableIdentifier.of(Namespace.of("namespace1"),
"table1");
+ TableIdentifier table2 = TableIdentifier.of(Namespace.of("namespace2"),
"table2");
+
+ String table1MetadataLocation =
createMetadataLocationViaJdbcCatalog(table1);
Review Comment:
I'm also not super opinionated since I see that the current test does
ultimately test the behavior we want to validate.
I think @nastra suggestion of
>get the legacy schema we might want to consider creating the table via the
Catalog API and then dropping the iceberg_type field.
is probably cleaner, because I think it removes the need for the insert
into statement with all the metadata file details etc. The only additional code
would be the one to drop the `iceberg_type` field but I think that would be
quite small in comparison (again all theoretical in my head :) ) ? So overall
seems like that approach would be a net reduction in the test code and still be
readable.
Either way not super opinionated, for the purpose of this PR I think we're
good; we can cleanup in a follow on.
--
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]
