ajantha-bhat commented on code in PR #10111:
URL: https://github.com/apache/iceberg/pull/10111#discussion_r1559338058
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcUtil.java:
##########
@@ -45,4 +51,97 @@ public void testFilterAndRemovePrefix() {
assertThat(expected).isEqualTo(actual);
}
+
+ @Test
+ public void testV0toV1SqlStatements() throws Exception {
+ java.nio.file.Path dbFile = Files.createTempFile("icebergSchemaUpdate",
"db");
+ String jdbcUrl = "jdbc:sqlite:" + dbFile.toAbsolutePath();
+
+ SQLiteDataSource dataSource = new SQLiteDataSource();
+ dataSource.setUrl(jdbcUrl);
+
+ try (JdbcClientPool connections = new JdbcClientPool(jdbcUrl,
Maps.newHashMap())) {
+ // create "old style" SQL schema
+
connections.newClient().prepareStatement(JdbcUtil.V0_CREATE_CATALOG_SQL).executeUpdate();
+
+ // inserting tables
+ JdbcUtil.doCommitCreateTable(
+ JdbcUtil.SchemaVersion.V0,
+ connections,
+ "TEST",
+ Namespace.of("namespace1"),
+ TableIdentifier.of(Namespace.of("namespace1"), "table1"),
+ "testLocation");
+ JdbcUtil.doCommitCreateTable(
+ JdbcUtil.SchemaVersion.V0,
+ connections,
+ "TEST",
+ Namespace.of("namespace1"),
+ TableIdentifier.of(Namespace.of("namespace1"), "table2"),
+ "testLocation");
+
+ try (PreparedStatement statement =
+
connections.newClient().prepareStatement(JdbcUtil.V0_LIST_TABLE_SQL)) {
+ statement.setString(1, "TEST");
+ statement.setString(2, "namespace1");
+ ResultSet tables = statement.executeQuery();
+ tables.next();
+ assertThat(tables.getString(JdbcUtil.TABLE_NAME)).isEqualTo("table1");
+ tables.next();
+ assertThat(tables.getString(JdbcUtil.TABLE_NAME)).isEqualTo("table2");
+ }
+
+ // updating the schema from V0 to V1
+
connections.newClient().prepareStatement(JdbcUtil.V1_UPDATE_CATALOG_SQL).execute();
+
+ // trying to add a table on the updated schema
+ JdbcUtil.doCommitCreateTable(
+ JdbcUtil.SchemaVersion.V1,
+ connections,
+ "TEST",
+ Namespace.of("namespace1"),
+ TableIdentifier.of(Namespace.of("namespace1"), "table3"),
+ "testLocation");
+
+ // testing the tables after migration and new table added
+ try (PreparedStatement statement =
+
connections.newClient().prepareStatement(JdbcUtil.V0_LIST_TABLE_SQL)) {
+ statement.setString(1, "TEST");
+ statement.setString(2, "namespace1");
+ ResultSet tables = statement.executeQuery();
+ tables.next();
+ assertThat(tables.getString(JdbcUtil.TABLE_NAME)).isEqualTo("table1");
+ assertThat(tables.getString(JdbcUtil.RECORD_TYPE)).isNull();
+ tables.next();
+ assertThat(tables.getString(JdbcUtil.TABLE_NAME)).isEqualTo("table2");
+ assertThat(tables.getString(JdbcUtil.RECORD_TYPE)).isNull();
+ tables.next();
+ assertThat(tables.getString(JdbcUtil.TABLE_NAME)).isEqualTo("table3");
+
assertThat(tables.getString(JdbcUtil.RECORD_TYPE)).isEqualTo(JdbcUtil.TABLE_RECORD_TYPE);
+ }
+
+ // update a table (commit) created on V1 schema
+ int updated =
+ JdbcUtil.updateTable(
+ JdbcUtil.SchemaVersion.V1,
+ connections,
+ "TEST",
+ TableIdentifier.of(Namespace.of("namespace1"), "table3"),
+ "newLocation",
+ "testLocation");
+ assertThat(updated).isEqualTo(1);
+
+ // update a table (commit) migrated from V0 schema
+ // this was failing before the fix from PR #10111
Review Comment:
nit: PR comment is enough for reviewer. Maybe no need of code comment.
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcUtil.java:
##########
@@ -45,4 +51,97 @@ public void testFilterAndRemovePrefix() {
assertThat(expected).isEqualTo(actual);
}
+
+ @Test
+ public void testV0toV1SqlStatements() throws Exception {
+ java.nio.file.Path dbFile = Files.createTempFile("icebergSchemaUpdate",
"db");
+ String jdbcUrl = "jdbc:sqlite:" + dbFile.toAbsolutePath();
+
+ SQLiteDataSource dataSource = new SQLiteDataSource();
+ dataSource.setUrl(jdbcUrl);
+
+ try (JdbcClientPool connections = new JdbcClientPool(jdbcUrl,
Maps.newHashMap())) {
+ // create "old style" SQL schema
+
connections.newClient().prepareStatement(JdbcUtil.V0_CREATE_CATALOG_SQL).executeUpdate();
+
+ // inserting tables
+ JdbcUtil.doCommitCreateTable(
+ JdbcUtil.SchemaVersion.V0,
+ connections,
+ "TEST",
+ Namespace.of("namespace1"),
+ TableIdentifier.of(Namespace.of("namespace1"), "table1"),
+ "testLocation");
+ JdbcUtil.doCommitCreateTable(
Review Comment:
nit: table2 may be redundant
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcUtil.java:
##########
@@ -45,4 +51,97 @@ public void testFilterAndRemovePrefix() {
assertThat(expected).isEqualTo(actual);
}
+
+ @Test
+ public void testV0toV1SqlStatements() throws Exception {
+ java.nio.file.Path dbFile = Files.createTempFile("icebergSchemaUpdate",
"db");
+ String jdbcUrl = "jdbc:sqlite:" + dbFile.toAbsolutePath();
+
+ SQLiteDataSource dataSource = new SQLiteDataSource();
+ dataSource.setUrl(jdbcUrl);
+
+ try (JdbcClientPool connections = new JdbcClientPool(jdbcUrl,
Maps.newHashMap())) {
+ // create "old style" SQL schema
+
connections.newClient().prepareStatement(JdbcUtil.V0_CREATE_CATALOG_SQL).executeUpdate();
+
+ // inserting tables
+ JdbcUtil.doCommitCreateTable(
+ JdbcUtil.SchemaVersion.V0,
+ connections,
+ "TEST",
+ Namespace.of("namespace1"),
+ TableIdentifier.of(Namespace.of("namespace1"), "table1"),
+ "testLocation");
+ JdbcUtil.doCommitCreateTable(
+ JdbcUtil.SchemaVersion.V0,
+ connections,
+ "TEST",
+ Namespace.of("namespace1"),
+ TableIdentifier.of(Namespace.of("namespace1"), "table2"),
+ "testLocation");
+
+ try (PreparedStatement statement =
+
connections.newClient().prepareStatement(JdbcUtil.V0_LIST_TABLE_SQL)) {
+ statement.setString(1, "TEST");
+ statement.setString(2, "namespace1");
+ ResultSet tables = statement.executeQuery();
+ tables.next();
+ assertThat(tables.getString(JdbcUtil.TABLE_NAME)).isEqualTo("table1");
+ tables.next();
+ assertThat(tables.getString(JdbcUtil.TABLE_NAME)).isEqualTo("table2");
+ }
+
+ // updating the schema from V0 to V1
+
connections.newClient().prepareStatement(JdbcUtil.V1_UPDATE_CATALOG_SQL).execute();
+
+ // trying to add a table on the updated schema
Review Comment:
can we add create one view after table3? and update it Because code is
changed for view commit SQL also and tests can cover it.
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java:
##########
@@ -54,7 +54,31 @@ enum SchemaVersion {
static final String TABLE_RECORD_TYPE = "TABLE";
static final String VIEW_RECORD_TYPE = "VIEW";
- private static final String V1_DO_COMMIT_SQL =
Review Comment:
Do we also need to handle the same for `V1_DO_COMMIT_CREATE_SQL` ?
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
