[ToBeSquashed]Address imjalpreet's comments

Author: hantangwangd

presto-iceberg/pom.xml

@@ -263,18 +263,6 @@
             </exclusions>
         </dependency>
 
-        <dependency>
-          <groupId>org.testcontainers</groupId>
-          <artifactId>testcontainers</artifactId>
-          <scope>test</scope>
-          <exclusions>
-            <exclusion>
-              <groupId>org.slf4j</groupId>
-              <artifactId>slf4j-api</artifactId>
-            </exclusion>
-          </exclusions>
-        </dependency>
-
         <dependency>
           <groupId>com.amazonaws</groupId>
           <artifactId>aws-java-sdk-core</artifactId>
@@ -620,6 +608,18 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+          <groupId>org.testcontainers</groupId>
+          <artifactId>testcontainers</artifactId>
+          <scope>test</scope>
+          <exclusions>
+            <exclusion>
+              <groupId>org.slf4j</groupId>
+              <artifactId>slf4j-api</artifactId>
+            </exclusion>
+          </exclusions>
+        </dependency>
+
         <dependency>
             <groupId>org.apache.iceberg</groupId>
             <artifactId>iceberg-core</artifactId>

presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java

@@ -1035,15 +1035,15 @@ public void setTableProperties(ConnectorSession session, ConnectorTableHandle ta
 
     protected Map<String, String> populateTableProperties(ConnectorTableMetadata tableMetadata, com.facebook.presto.iceberg.FileFormat fileFormat, ConnectorSession session, CatalogType catalogType)
     {
-        ImmutableMap.Builder<String, String> propertiesBuilder = ImmutableMap.builderWithExpectedSize(5);
+        ImmutableMap.Builder<String, String> propertiesBuilder = ImmutableMap.builderWithExpectedSize(16);
 
         String writeDataLocation = IcebergTableProperties.getWriteDataLocation(tableMetadata.getProperties());
         if (!Strings.isNullOrEmpty(writeDataLocation)) {
             if (catalogType.equals(CatalogType.HADOOP)) {
                 propertiesBuilder.put(WRITE_DATA_LOCATION, writeDataLocation);
             }
             else {
-                throw new PrestoException(NOT_SUPPORTED, "Not support set write_data_path on catalog: " + catalogType);
+                throw new PrestoException(NOT_SUPPORTED, "Table property write_data_path is not supported with catalog type: " + catalogType);
             }
         }
         else {

presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergCommonModule.java

@@ -95,6 +95,8 @@
 import static com.facebook.airlift.concurrent.Threads.daemonThreadsNamed;
 import static com.facebook.airlift.configuration.ConfigBinder.configBinder;
 import static com.facebook.airlift.json.JsonCodecBinder.jsonCodecBinder;
+import static com.facebook.presto.common.Utils.checkArgument;
+import static com.facebook.presto.iceberg.CatalogType.HADOOP;
 import static com.facebook.presto.orc.StripeMetadataSource.CacheableRowGroupIndices;
 import static com.facebook.presto.orc.StripeMetadataSource.CacheableSlice;
 import static com.google.common.util.concurrent.MoreExecutors.newDirectExecutorService;
@@ -137,6 +139,9 @@ protected void setup(Binder binder)
 
         configBinder(binder).bindConfig(IcebergConfig.class);
 
+        IcebergConfig icebergConfig = buildConfigObject(IcebergConfig.class);
+        checkArgument(icebergConfig.getCatalogType().equals(HADOOP) || icebergConfig.getCatalogWarehouseDataDir() == null, "'iceberg.catalog.warehouse.datadir' can only be specified in Hadoop catalog");
+
         binder.bind(IcebergSessionProperties.class).in(Scopes.SINGLETON);
         newOptionalBinder(binder, IcebergNessieConfig.class);  // bind optional Nessie config to IcebergSessionProperties
 

presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergConfig.java

@@ -134,7 +134,7 @@ public String getCatalogWarehouseDataDir()
     }
 
     @Config("iceberg.catalog.warehouse.datadir")
-    @ConfigDescription("Iceberg catalog default root data writing directory")
+    @ConfigDescription("Iceberg catalog default root data writing directory. This is only supported with Hadoop catalog.")
     public IcebergConfig setCatalogWarehouseDataDir(String catalogWarehouseDataDir)
     {
         this.catalogWarehouseDataDir = catalogWarehouseDataDir;

presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java

@@ -158,16 +158,16 @@ public void testTableWithSpecifiedWriteDataLocation()
     {
         String dataWriteLocation = Files.createTempDirectory("test_table_with_specified_write_data_location2").toAbsolutePath().toString();
         assertQueryFails(String.format("create table test_table_with_specified_write_data_location2(a int, b varchar) with (write_data_path = '%s')", dataWriteLocation),
-                "Not support set write_data_path on catalog: " + catalogType);
+                "Table property write_data_path is not supported with catalog type: " + catalogType);
     }
 
     @Test
     public void testPartitionedTableWithSpecifiedWriteDataLocation()
             throws IOException
     {
-        String dataWriteLocation = Files.createTempDirectory("test_table_with_specified_write_data_location3").toAbsolutePath().toString();
-        assertQueryFails(String.format("create table test_table_with_specified_write_data_location3(a int, b varchar) with (write_data_path = '%s')", dataWriteLocation),
-                "Not support set write_data_path on catalog: " + catalogType);
+        String dataWriteLocation = Files.createTempDirectory("test_partitioned_table_with_specified_write_data_location").toAbsolutePath().toString();
+        assertQueryFails(String.format("create table test_partitioned_table_with_specified_write_data_location(a int, b varchar) with (write_data_path = '%s', partitioning = ARRAY['a'])", dataWriteLocation),
+                "Table property write_data_path is not supported with catalog type: " + catalogType);
     }
 
     @Test
@@ -177,7 +177,7 @@ public void testShowCreateTableWithSpecifiedWriteDataLocation()
         String tableName = "test_table_with_specified_write_data_location";
         String dataWriteLocation = java.nio.file.Files.createTempDirectory("test1").toAbsolutePath().toString();
         assertQueryFails(format("CREATE TABLE %s(a int, b varchar) with (write_data_path = '%s')", tableName, dataWriteLocation),
-                "Not support set write_data_path on catalog: " + catalogType);
+                "Table property write_data_path is not supported with catalog type: " + catalogType);
     }
 
     @Test

presto-iceberg/src/test/java/com/facebook/presto/iceberg/hadoop/TestIcebergSmokeHadoop.java

@@ -99,7 +99,7 @@ public void testTableWithSpecifiedWriteDataLocation()
     public void testPartitionedTableWithSpecifiedWriteDataLocation()
             throws IOException
     {
-        String tableName = "test_table_with_specified_write_data_location3";
+        String tableName = "test_partitioned_table_with_specified_write_data_location";
         String dataWriteLocation = Files.createTempDirectory(tableName).toAbsolutePath().toString();
         try {
             assertUpdate(format("create table %s(a int, b varchar) with (partitioning = ARRAY['a'], write_data_path = '%s')", tableName, dataWriteLocation));