jerryshao commented on code in PR #13689:
URL: https://github.com/apache/gravitino/pull/13689#discussion_r4214965370


##########
core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java:
##########
@@ -255,6 +259,219 @@ public void testUpgradeToTwoZeroBackfillsOccVersions() 
throws SQLException, IOEx
     }
   }
 
+  /** Verifies a completed upgrade can be repeated without resetting migrated 
data. */
+  @TestTemplate
+  public void testUpgradeToTwoZeroIsIdempotent() throws SQLException, 
IOException {
+    Path scriptDir = upgradeScriptDirectory();
+    String suffix = "-" + backendType.toLowerCase() + ".sql";
+    File upgrade = scriptDir.resolve("upgrade-1.3.0-to-2.0.0" + 
suffix).toFile();
+    dropAllTables();
+    executeScript(scriptDir.resolve("schema-1.3.0" + suffix).toFile());
+    seedUpgradeData();
+    executeScript(upgrade);
+    executeStatements(
+        List.of(
+            "UPDATE idp_user_meta SET enabled = FALSE, audit_info = 'preserved 
audit'",
+            "UPDATE idp_group_meta SET group_comment = 'preserved comment'",
+            "UPDATE tag_meta SET allowed_values = '[\"blue\"]'",
+            "UPDATE tag_relation_meta SET tag_value = 'blue'",
+            "UPDATE fileset_meta SET occ_version = 42",
+            "UPDATE policy_meta SET occ_version = 43"),
+        "customized migrated data");
+    Map<String, Set<String>> expectedColumns = readSchemaColumns();
+    List<String> expectedIndexes = readSchemaIndexes();
+    Map<String, List<List<String>>> expectedData = readUpgradeData();
+    executeScript(upgrade);
+    executeScript(upgrade);
+    Assertions.assertEquals(expectedColumns, readSchemaColumns());
+    Assertions.assertEquals(expectedIndexes, readSchemaIndexes());
+    Assertions.assertEquals(expectedData, readUpgradeData(), "Retry must 
preserve migrated values");
+    if ("mysql".equals(backendType)) {
+      // Neither rename source nor target exists: this is damage, not an 
already-applied DDL.
+      executeStatements(
+          List.of("ALTER TABLE schema_meta DROP INDEX schema_meta_idx_mid"),
+          "missing target index");
+      AssertionFailedError failure =
+          Assertions.assertThrows(AssertionFailedError.class, () -> 
executeScript(upgrade));
+      Assertions.assertInstanceOf(SQLException.class, failure.getCause());
+      Assertions.assertEquals(1176, ((SQLException) 
failure.getCause()).getErrorCode());
+    }
+  }
+
+  /** Verifies retries after structural DDL and data updates, including 
prepared DDL execution. */
+  @TestTemplate
+  public void testUpgradeToTwoZeroRetriesPartialExecution() throws 
SQLException, IOException {
+    Path scriptDir = upgradeScriptDirectory();
+    String suffix = "-" + backendType.toLowerCase() + ".sql";
+    File schema = scriptDir.resolve("schema-1.3.0" + suffix).toFile();
+    File upgrade = scriptDir.resolve("upgrade-1.3.0-to-2.0.0" + 
suffix).toFile();
+    List<String> statements = extractStatements(upgrade.toPath());
+    dropAllTables();
+    executeScript(schema);
+    seedUpgradeData();
+    executeScript(upgrade);
+    Map<String, Set<String>> expectedColumns = readSchemaColumns();
+    List<String> expectedIndexes = readSchemaIndexes();
+    Map<String, List<List<String>>> expectedData = readUpgradeData();
+
+    for (int completed = 1; completed <= statements.size(); completed++) {
+      String last = statements.get(completed - 1).toUpperCase();
+      if (!(last.startsWith("ALTER TABLE")
+          || last.startsWith("CREATE ")
+          || last.startsWith("UPDATE ")
+          || last.startsWith("EXECUTE "))) {
+        continue;
+      }
+      dropAllTables();
+      executeScript(schema);
+      seedUpgradeData();
+      // Close the connection after the prefix, as if the migration process 
stopped here.
+      String checkpoint = "Interrupted after statement " + completed + ": " + 
last;
+      executeStatements(statements.subList(0, completed), checkpoint);
+      executeScript(upgrade);
+      Assertions.assertEquals(expectedColumns, readSchemaColumns(), 
checkpoint);
+      Assertions.assertEquals(expectedIndexes, readSchemaIndexes(), 
checkpoint);
+      Assertions.assertEquals(expectedData, readUpgradeData(), checkpoint);
+      if ("mysql".equals(backendType)) {
+        try (SqlSession session =
+                
SqlSessionFactoryHelper.getInstance().getSqlSessionFactory().openSession(true);
+            Connection connection = session.getConnection();
+            ResultSet keys =
+                connection
+                    .getMetaData()
+                    .getPrimaryKeys(connection.getCatalog(), null, 
"table_version_info")) {
+          List<String> columns = new ArrayList<>();
+          while (keys.next()) {
+            columns.add(keys.getString("COLUMN_NAME"));
+          }
+          Assertions.assertEquals(
+              Set.of("table_id", "version", "deleted_at"), 
Set.copyOf(columns), checkpoint);
+        }
+      }
+    }
+  }
+
+  private Path upgradeScriptDirectory() {
+    String home = System.getenv("GRAVITINO_HOME");
+    Assertions.assertNotNull(home, "GRAVITINO_HOME environment variable is not 
set");
+    return Path.of(home, "scripts", backendType.toLowerCase());
+  }
+
+  private void seedUpgradeData() throws SQLException {
+    executeStatements(
+        List.of(
+            "INSERT INTO idp_user_meta (user_id, user_name, password_hash, 
current_version, last_version) VALUES (1, 'user', 'hash', 7, 9)",
+            "INSERT INTO idp_group_meta (group_id, group_name) VALUES (1, 
'group')",
+            "INSERT INTO tag_meta (tag_id, tag_name, metalake_id, audit_info) 
VALUES (1, 'tag', 1, '{}')",
+            "INSERT INTO tag_relation_meta (id, tag_id, metadata_object_id, 
metadata_object_type, audit_info) VALUES (1, 1, 1, 'TABLE', '{}')",
+            "INSERT INTO fileset_meta (fileset_id, fileset_name, metalake_id, 
catalog_id, schema_id, type, audit_info, current_version, last_version) VALUES 
(1, 'fileset', 1, 1, 1, 'MANAGED', '{}', 7, 9)",
+            "INSERT INTO policy_meta (policy_id, policy_name, policy_type, 
metalake_id, audit_info, current_version, last_version) VALUES (1, 'policy', 
'custom', 1, '{}', 7, 9)",
+            "INSERT INTO table_version_info (table_id, version, deleted_at) 
VALUES (1, 7, 0)"),

Review Comment:
   Can you fix the code length here?



-- 
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]

Reply via email to