haizhou-zhao commented on code in PR #11093:
URL: https://github.com/apache/iceberg/pull/11093#discussion_r1800188393


##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestBaseWithCatalog.java:
##########
@@ -89,13 +138,37 @@ public static void dropWarehouse() throws IOException {
   protected TableIdentifier tableIdent = 
TableIdentifier.of(Namespace.of("default"), "table");
   protected String tableName;
 
+  private void configureValidationCatalog() {
+    if (catalogConfig.containsKey(ICEBERG_CATALOG_TYPE)) {
+      switch (catalogConfig.get(ICEBERG_CATALOG_TYPE)) {
+        case ICEBERG_CATALOG_TYPE_HADOOP:
+          this.validationCatalog =
+              new HadoopCatalog(spark.sessionState().newHadoopConf(), "file:" 
+ warehouse);
+          break;
+        case ICEBERG_CATALOG_TYPE_REST:
+          this.validationCatalog = restCatalog;
+          break;
+        case ICEBERG_CATALOG_TYPE_HIVE:
+          this.validationCatalog = catalog;
+          break;
+        default:
+          throw new IllegalArgumentException("Unknown catalog type");
+      }
+    } else if (catalogConfig.containsKey(CATALOG_IMPL)) {
+      switch (catalogConfig.get(CATALOG_IMPL)) {
+        case "org.apache.iceberg.inmemory.InMemoryCatalog":
+          this.validationCatalog = new InMemoryCatalog();
+          break;
+        default:
+          throw new IllegalArgumentException("Unknown catalog impl");
+      }
+    }
+    this.validationNamespaceCatalog = (SupportsNamespaces) validationCatalog;
+  }
+
   @BeforeEach
   public void before() {
-    this.validationCatalog =

Review Comment:
   If I understand correctly, the original `validationCatalog` is set to a 
`HadoopCatalog` if `catalogName` of the catalog being tested is named 
`testhadoop`; otherwise, make the `validationCatalog` the same as `catalog` 
(which is strictly a `HiveCatalog`, as defined by `TestBase` class).
   
   That will work in the old days, as we only have 2 types catalogs, either 
Hadoop or Hive, being tested - if the test subject is not a Hadoop Catalog, 
then setting the validation catalog to Hive Catalog will suffice the validation 
purpose. However, with the introduction of a 3rd type, REST catalog: when the 
test subject is a REST catalog, without changing how `validationCatalog` is 
initialized, the `validationCatalog` will be set to a Hive catalog. In this 
case, conducting test behaviors on REST catalog while validating the status 
post-change on Hive catalog won't work.
   
   Unless, you are suggesting that we should make changes to `TestBase` class 
where the `catalog` being tested does not strictly need to be a HiveCatalog, 
and can be any type of catalog.



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

Reply via email to