Address comments

prestodb · Feb 7, 2025 · 04f8aaa · 04f8aaa
1 parent 9c3402b
commit 04f8aaa
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 11 deletions.
diff --git a/presto-docs/src/main/sphinx/connector/iceberg.rst b/presto-docs/src/main/sphinx/connector/iceberg.rst
@@ -389,7 +389,7 @@ Property Name                             Description
 ``metrics_max_inferred_column``            Optionally specifies the maximum number of columns for which     ``100``
                                            metrics are collected.
 
-``read.target.split-size``                 The target size for an individual split when generating splits     ``134217728`` (128MB)
+``read.split.target-size``                 The target size for an individual split when generating splits     ``134217728`` (128MB)
                                            for a table scan. Must be specified in bytes.
 =======================================   ===============================================================   =========================
 
@@ -426,7 +426,7 @@ Property Name                                         Description
                                                       session.
 ``iceberg.target_split_size``                         Overrides the target split size for all tables in a query in bytes.
                                                       Set to 0 to use the value in each Iceberg table's
-                                                      ``read.target.split-size`` property.
+                                                      ``read.split.target-size`` property.
 ===================================================== ======================================================================
 
 Caching Support

diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergUtil.java
@@ -159,6 +159,7 @@
 import static com.google.common.collect.Streams.stream;
 import static io.airlift.slice.Slices.utf8Slice;
 import static io.airlift.slice.Slices.wrappedBuffer;
+import static io.airlift.units.DataSize.succinctBytes;
 import static java.lang.Double.doubleToRawLongBits;
 import static java.lang.Double.longBitsToDouble;
 import static java.lang.Double.parseDouble;
@@ -1284,9 +1285,9 @@ public static Long getSplitSize(Table table)
 
     public static DataSize getTargetSplitSize(long sessionValueProperty, long icebergScanTargetSplitSize)
     {
-        return Optional.of(DataSize.succinctBytes(sessionValueProperty))
-                .filter(size -> !size.equals(DataSize.succinctBytes(0)))
-                .orElse(DataSize.succinctBytes(icebergScanTargetSplitSize));
+        return sessionValueProperty == 0 ?
+                succinctBytes(icebergScanTargetSplitSize) :
+                succinctBytes(sessionValueProperty);
     }
 
     public static DataSize getTargetSplitSize(ConnectorSession session, Scan<?, ?, ?> scan)

diff --git a/...rg/src/test/java/com/facebook/presto/iceberg/procedure/TestSetTablePropertyProcedure.java b/...rg/src/test/java/com/facebook/presto/iceberg/procedure/TestSetTablePropertyProcedure.java
@@ -33,6 +33,7 @@
 import static com.facebook.presto.iceberg.IcebergQueryRunner.createIcebergQueryRunner;
 import static com.facebook.presto.iceberg.IcebergQueryRunner.getIcebergDataDirectoryPath;
 import static java.lang.String.format;
+import static org.apache.iceberg.TableProperties.SPLIT_SIZE_DEFAULT;
 import static org.testng.Assert.assertEquals;
 
 public class TestSetTablePropertyProcedure
@@ -64,21 +65,21 @@ public void testSetTablePropertyProcedurePositionalArgs()
         String tableName = "table_property_table_test";
         createTable(tableName);
         try {
-            String propertyKey = "read.split.planning-lookback";
+            String propertyKey = "read.split.target-size";
             String propertyValue = "268435456";
             assertUpdate("INSERT INTO " + tableName + " VALUES (1, 'a')", 1);
 
             Table table = loadTable(tableName);
             table.refresh();
 
             assertEquals(table.properties().size(), 8);
-            assertEquals(table.properties().get(propertyKey), null);
+            assertEquals(Long.parseLong(table.properties().get(propertyKey)), SPLIT_SIZE_DEFAULT);
 
             assertUpdate(format("CALL system.set_table_property('%s', '%s', '%s', '%s')", TEST_SCHEMA, tableName, propertyKey, propertyValue));
             table.refresh();
 
             // now the table property read.split.target-size should have new value
-            assertEquals(table.properties().size(), 9);
+            assertEquals(table.properties().size(), 8);
             assertEquals(table.properties().get(propertyKey), propertyValue);
         }
         finally {
@@ -92,22 +93,22 @@ public void testSetTablePropertyProcedureNamedArgs()
         String tableName = "table_property_table_arg_test";
         createTable(tableName);
         try {
-            String propertyKey = "read.split.planning-lookback";
+            String propertyKey = "read.split.target-size";
             String propertyValue = "268435456";
             assertUpdate("INSERT INTO " + tableName + " VALUES (1, 'a')", 1);
 
             Table table = loadTable(tableName);
             table.refresh();
 
             assertEquals(table.properties().size(), 8);
-            assertEquals(table.properties().get(propertyKey), null);
+            assertEquals(Long.parseLong(table.properties().get(propertyKey)), SPLIT_SIZE_DEFAULT);
 
             assertUpdate(format("CALL system.set_table_property(schema => '%s', key => '%s', value => '%s', table_name => '%s')",
                     TEST_SCHEMA, propertyKey, propertyValue, tableName));
             table.refresh();
 
             // now the table property read.split.target-size should have new value
-            assertEquals(table.properties().size(), 9);
+            assertEquals(table.properties().size(), 8);
             assertEquals(table.properties().get(propertyKey), propertyValue);
         }
         finally {