amogh-jahagirdar commented on code in PR #9274:
URL: https://github.com/apache/iceberg/pull/9274#discussion_r1426150782


##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -351,6 +351,8 @@ private Map<String, String> summary(TableMetadata previous) 
{
         SnapshotSummary.ADDED_EQ_DELETES_PROP,
         SnapshotSummary.REMOVED_EQ_DELETES_PROP);
 
+    builder.putAll(EnvironmentContext.get());
+

Review Comment:
   Is this for another change? I don't see how it relates to this PR



##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java:
##########
@@ -974,6 +974,28 @@ public void 
testPartitionedImportFromEmptyPartitionDoesNotThrow() {
         sql("SELECT * FROM %s ORDER BY id", tableName));
   }
 
+  @Test
+  public void addDataParallelListing() {

Review Comment:
   Nit: If we could follow the existing naming convention in the test class it 
helps readability. `testAddFilesWithParallelListing`



##########
docs/spark-procedures.md:
##########
@@ -639,6 +639,7 @@ Keep in mind the `add_files` procedure will fetch the 
Parquet metadata from each
 | `source_table`          | ✔️        | string              | Table where 
files should come from, paths are also possible in the form of 
\`file_format\`.\`path\` |
 | `partition_filter`      | ️         | map<string, string> | A map of 
partitions in the source table to import from                                   
           |
 | `check_duplicate_files` | ️         | boolean             | Whether to 
prevent files existing in the table from being added (defaults to true)         
         |
+| `listing_parallelism`   |           | int                 | The parallelism 
to use when listing files (defaults to 1)                                       
    |

Review Comment:
   Thanks for adding docs!



##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/procedures/ProcedureInput.java:
##########
@@ -68,6 +68,18 @@ public Boolean asBoolean(ProcedureParameter param, Boolean 
defaultValue) {
     return args.isNullAt(ordinal) ? defaultValue : (Boolean) 
args.getBoolean(ordinal);
   }
 
+  public Integer asInt(ProcedureParameter param) {
+    Integer value = asInt(param, null);
+    Preconditions.checkArgument(value != null, "Parameter '%s' is not set", 
param.name());
+    return value;
+  }
+
+  public Integer asInt(ProcedureParameter param, Integer defaultValue) {
+    validateParamType(param, DataTypes.IntegerType);

Review Comment:
   Does this validation fail if there's a value which overflows int?



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