ismailsimsek commented on a change in pull request #1870:
URL: https://github.com/apache/iceberg/pull/1870#discussion_r637403676



##########
File path: 
spark3/src/test/java/org/apache/iceberg/actions/TestRemoveOrphanFilesAction3.java
##########
@@ -165,4 +165,64 @@ public void testSparkSessionCatalogHiveTable() throws 
Exception {
     spark.conf().unset("spark.sql.catalog.spark_catalog.type");
   }
 
+  @Test
+  public void testSparkCatalogNamedJdbcTable() throws Exception {
+    spark.conf().set("spark.sql.catalog.jdbc", 
"org.apache.iceberg.spark.SparkCatalog");
+    spark.conf().set("spark.sql.catalog.jdbc.type", "jdbc");
+    spark.conf().set("spark.sql.catalog.jdbc.warehouse", tableLocation);
+    spark.conf().set("spark.sql.catalog.jdbc.uri", 
"jdbc:sqlite:file::memory:?icsparkjdbctestcat;");
+    spark.conf().set("spark.sql.catalog.jdbc.connection.parameter.user", 
"testuser");
+    spark.conf().set("spark.sql.catalog.jdbc.connection.parameter.password", 
"testpassword");
+    SparkCatalog cat = (SparkCatalog) 
spark.sessionState().catalogManager().catalog("jdbc");
+
+    String[] database = {"default"};
+    Identifier id = Identifier.of(database, "table");
+    Map<String, String> options = Maps.newHashMap();
+    Transform[] transforms = {};
+    cat.createTable(id, SparkSchemaUtil.convert(SCHEMA), transforms, options);
+    SparkTable table = cat.loadTable(id);
+
+    spark.sql("INSERT INTO jdbc.default.table VALUES (1,1,1)");
+
+    String location = table.table().location().replaceFirst("file:", "");
+    new File(location + "/data/trashfile").createNewFile();
+
+    List<String> results = Actions.forTable(table.table()).removeOrphanFiles()
+        .olderThan(System.currentTimeMillis() + 1000).execute();
+    Assert.assertTrue("trash file should be removed",
+        results.contains("file:" + location + "/data/trashfile"));
+  }
+
+  @Test
+  public void testSparkSessionCatalogJdbcTable() throws Exception {
+    spark.conf().set("spark.sql.catalog.spark_catalog", 
"org.apache.iceberg.spark.SparkSessionCatalog");
+    spark.conf().set("spark.sql.catalog.spark_catalog.type", "jdbc");
+    spark.conf().set("spark.sql.catalog.spark_catalog.warehouse", 
tableLocation);
+    spark.conf().set("spark.sql.catalog.spark_catalog.uri", 
"jdbc:sqlite:file::memory:?icsparktestcat;");
+    
spark.conf().set("spark.sql.catalog.spark_catalog.connection.parameter.user", 
"testuser");
+    
spark.conf().set("spark.sql.catalog.spark_catalog.connection.parameter.password",
 "testpassword");
+    SparkSessionCatalog cat = (SparkSessionCatalog) 
spark.sessionState().catalogManager().v2SessionCatalog();
+
+    String[] database = {"default"};
+    Identifier id = Identifier.of(database, "sessioncattest");
+    Map<String, String> options = Maps.newHashMap();
+    Transform[] transforms = {};
+    cat.dropTable(id);
+    cat.createTable(id, SparkSchemaUtil.convert(SCHEMA), transforms, options);
+    SparkTable table = (SparkTable) cat.loadTable(id);
+
+    spark.sql("INSERT INTO default.sessioncattest VALUES (1,1,1)");
+
+    String location = table.table().location().replaceFirst("file:", "");
+    new File(location + "/data/trashfile").createNewFile();
+
+    List<String> results = Actions.forTable(table.table()).removeOrphanFiles()
+        .olderThan(System.currentTimeMillis() + 1000).execute();
+    Assert.assertTrue("trash file should be removed",
+        results.contains("file:" + location + "/data/trashfile"));
+    // reset spark_catalog to default

Review comment:
       after rolling back CatalogUtil change i believe jdbc related test are 
not necessary, so rolled back the changes, it was duplication of existing 
tests, probably we could add better spark-jdbc-catalog tests with new PR? what 
do you think? 
   
   moved `reset...xxx` section to `resetSparkSessionCatalog` using `@After`




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to