[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #754: IGNITE-16691 Storage Configuration refactoring and improvements

[GitBox <gi...@apache.org>]

tkalkirill commented on code in PR #754:
URL: https://github.com/apache/ignite-3/pull/754#discussion_r841739612


##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java:
##########
@@ -63,70 +58,26 @@ public void validate(TableValidator annotation, ValidationContext<NamedListView<
                         + newTable.columns().size() + " columns"));
             }
 
-            validateDataRegion(oldTables == null ? null : oldTables.get(tableName), newTable, ctx);
+            validateDataStorage(oldTables == null ? null : oldTables.get(tableName), newTable, ctx);
 
             validateNamedListKeys(newTable, ctx);
         }
     }
 
-    /**
-     * Checks that data region configuration is valid. Check involves data region existence and region type preservation if it's updated.
-     *
-     * @param oldTable Previous configuration, maybe {@code null}.
-     * @param newTable New configuration.
-     * @param ctx      Validation context.
-     */
-    private static void validateDataRegion(@Nullable TableView oldTable, TableView newTable, ValidationContext<?> ctx) {
-        DataStorageView oldDbCfg = ctx.getOldRoot(DataStorageConfiguration.KEY);
-        DataStorageView newDbCfg = ctx.getNewRoot(DataStorageConfiguration.KEY);
-
-        if (oldTable != null && Objects.equals(oldTable.dataRegion(), newTable.dataRegion())) {
-            return;
-        }
-
-        DataRegionView newRegion = dataRegion(newDbCfg, newTable.dataRegion());
-
-        if (newRegion == null) {
-            ctx.addIssue(new ValidationIssue(String.format(
-                    "Data region '%s' configured for table '%s' isn't found",
-                    newTable.dataRegion(),
-                    newTable.name()
-            )));
-
-            return;
-        }
-
-        if (oldDbCfg == null || oldTable == null) {
-            return;
-        }
-
-        DataRegionView oldRegion = dataRegion(oldDbCfg, oldTable.dataRegion());
-
-        if (!oldRegion.type().equalsIgnoreCase(newRegion.type())) {
-            ctx.addIssue(new ValidationIssue(String.format(
-                    "Unable to move table '%s' from region '%s' to region '%s' because it has different type (old=%s, new=%s)",
-                    newTable.name(),
-                    oldTable.dataRegion(),
-                    newTable.dataRegion(),
-                    oldRegion.type(),
-                    newRegion.type()
-            )));
-        }
-    }
-
-    /**
-     * Retrieves data region configuration.
-     *
-     * @param dbCfg      Data storage configuration.
-     * @param regionName Data region name.
-     * @return Data region configuration.
-     */
-    private static DataRegionView dataRegion(DataStorageView dbCfg, String regionName) {
-        if (regionName.equals(DEFAULT_DATA_REGION_NAME)) {
-            return dbCfg.defaultRegion();
+    private static void validateDataStorage(@Nullable TableView oldTable, TableView newTable, ValidationContext<?> ctx) {
+        if (oldTable != null) {
+            String oldDataStorage = oldTable.dataStorage().name();

Review Comment:
   This is a **DataStorageConfigurationSchema#name** with **@PolymorphicId**, I think you can leave the validation here, wdyt?



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org