You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "slfan1989 (via GitHub)" <gi...@apache.org> on 2025/04/12 02:30:36 UTC
[PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
slfan1989 opened a new pull request, #12784:
URL: https://github.com/apache/iceberg/pull/12784
Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#discussion_r2043415467
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java:
##########
@@ -1109,6 +1109,22 @@ public void testAddFilesPartitionedWithParallelism() {
sql("SELECT id, name, dept, subdept FROM %s ORDER BY id", tableName));
}
+ @TestTemplate
+ public void testAddFilesWithInvalidParallelism() {
+ createUnpartitionedHiveTable();
+
+ createIcebergTable(
+ "id Integer, name String, dept String, subdept String", "PARTITIONED BY (id)");
+
+ assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.add_files(table => '%s', source_table => '%s', parallelism => -1)",
Review Comment:
nit: Testing with the boundary value `0` is more natural. I know `testMigrateWithInvalidParallelism` uses `-1`, though.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#discussion_r2041361102
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java:
##########
@@ -1109,6 +1110,22 @@ public void testAddFilesPartitionedWithParallelism() {
sql("SELECT id, name, dept, subdept FROM %s ORDER BY id", tableName));
}
+ @TestTemplate
+ public void testMigrateWithInvalidParallelism() throws IOException {
+ createUnpartitionedHiveTable();
+
+ createIcebergTable(
+ "id Integer, name String, dept String, subdept String", "PARTITIONED BY (id)");
+
+ assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.add_files(table => '%s', source_table => '%s', parallelism => %d)",
+ catalogName, tableName, sourceTableName, -1))
Review Comment:
Thank you for your suggestions. I have already made the changes to the code.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#issuecomment-2817736237
@slfan1989 It requires an approval from a committer.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #12784:
URL: https://github.com/apache/iceberg/pull/12784
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#discussion_r2041358636
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java:
##########
@@ -1109,6 +1110,22 @@ public void testAddFilesPartitionedWithParallelism() {
sql("SELECT id, name, dept, subdept FROM %s ORDER BY id", tableName));
}
+ @TestTemplate
+ public void testMigrateWithInvalidParallelism() throws IOException {
Review Comment:
@ebyhr Thanks for reviewing the code! I agree with your suggestions.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#issuecomment-2817756416
> @slfan1989 It requires an approval from a committer.
@manuzhang Thank you for your message! @nastra Could you take a look at this PR? Thank you very much!
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#issuecomment-2817672735
@manuzhang Thank you for the review and approval! If there are no further issues, could you please merge this PR?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#discussion_r2041101703
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java:
##########
@@ -1109,6 +1110,22 @@ public void testAddFilesPartitionedWithParallelism() {
sql("SELECT id, name, dept, subdept FROM %s ORDER BY id", tableName));
}
+ @TestTemplate
+ public void testMigrateWithInvalidParallelism() throws IOException {
+ createUnpartitionedHiveTable();
+
+ createIcebergTable(
+ "id Integer, name String, dept String, subdept String", "PARTITIONED BY (id)");
+
+ assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.add_files(table => '%s', source_table => '%s', parallelism => %d)",
+ catalogName, tableName, sourceTableName, -1))
Review Comment:
nit: We can inline `-1`.
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java:
##########
@@ -1109,6 +1110,22 @@ public void testAddFilesPartitionedWithParallelism() {
sql("SELECT id, name, dept, subdept FROM %s ORDER BY id", tableName));
}
+ @TestTemplate
+ public void testMigrateWithInvalidParallelism() throws IOException {
Review Comment:
```suggestion
public void testAddFilesWithInvalidParallelism() {
```
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#issuecomment-2803523254
@manuzhang @ebyhr Could you help review this PR? Thank you very much!
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add Parallelism Parameter Validation to AddFilesProcedure. [iceberg]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #12784:
URL: https://github.com/apache/iceberg/pull/12784#issuecomment-2820537106
@nastra @manuzhang @ebyhr Thank you very much for the review!
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org