kbendick commented on a change in pull request #2911:
URL: https://github.com/apache/iceberg/pull/2911#discussion_r681217709
##########
File path:
spark3-extensions/src/test/java/org/apache/iceberg/spark/extensions/SparkRowLevelOperationsTestBase.java
##########
@@ -67,14 +72,16 @@ public SparkRowLevelOperationsTestBase(String catalogName,
String implementation
"default-namespace", "default"
),
"orc",
- true
+ true,
+ "none"
Review comment:
Nit: I believe that there are constants for these, could you use those
instead? In case the string value is ever updated etc =)
##########
File path:
spark3-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestDelete.java
##########
@@ -59,8 +59,8 @@
public abstract class TestDelete extends SparkRowLevelOperationsTestBase {
public TestDelete(String catalogName, String implementation, Map<String,
String> config,
- String fileFormat, Boolean vectorized) {
- super(catalogName, implementation, config, fileFormat, vectorized);
+ String fileFormat, Boolean vectorized, String
distributionMode) {
+ super(catalogName, implementation, config, fileFormat, vectorized,
distributionMode);
Review comment:
So as to minimize the diff in this PR (larger diffs can make it harder
to backport these changes if people have forks), would it make sense to add a
second constructor that has `distributionMode` defaulted, so that only the
tests that need to pass `distributionMode` have their files updated in this PR?
Not sure if the change would be here or `SparkRowLevelOperationsTestBase`,
will leave that up to you =)
If only a few places were updated to use the constructor and don't make use
of the `distributionMode`, you can ignore / resolve this =)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
