You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "jerqi (via GitHub)" <gi...@apache.org> on 2023/08/31 03:05:37 UTC
[GitHub] [iceberg] jerqi opened a new pull request, #8438: Delete fix
jerqi opened a new pull request, #8438:
URL: https://github.com/apache/iceberg/pull/8438
(no comment)
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311298908
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
+ withSQLConf(
+ (Map<String, String>) propertiesSuite[0],
+ () -> {
+ Table table = validationCatalog.loadTable(tableIdent);
+ Map<String, String> writeOptions = ImmutableMap.of();
+ Map<String, String> tableProperties = (Map<String, String>) propertiesSuite[1];
+ UpdateProperties updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.set(entry.getKey(), entry.getValue());
+ }
+
+ updateProperties.commit();
+
+ SparkWriteConf writeConf = new SparkWriteConf(spark, table, writeOptions);
+ Map<String, String> writeProperties = writeConf.writeProperties();
+ Map<String, String> expectedProperties = (Map<String, String>) propertiesSuite[2];
+ Assert.assertEquals(expectedProperties.size(), writeConf.writeProperties().size());
+ for (Map.Entry<String, String> entry : writeProperties.entrySet()) {
+ Assert.assertEquals(entry.getValue(), expectedProperties.get(entry.getKey()));
+ }
+
+ table.refresh();
+ updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.remove(entry.getKey());
+ }
+
+ updateProperties.commit();
+ });
+ }
+
+ private Object[][] getOverridePropertiesSuites() {
+ return new Object[][] {
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")
+ },
+ {
+ ImmutableMap.of(
+ COMPRESSION_CODEC, "zstd", SparkSQLProperties.COMPRESSION_STRATEGY, "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")
+ },
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ };
+ }
+
+ private Object[][] getNonOverridePropertiesSuite() {
+ return new Object[][] {
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "5")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "speed")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
Review Comment:
case 3: The properties of delete file format exists.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311056981
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java:
##########
@@ -425,7 +434,85 @@ public String branch() {
return branch;
}
- public String parquetCompressionCodec() {
+ public Map<String, String> writeProperties(FileFormat dataFormat, FileFormat deleteFormat) {
Review Comment:
I will add a ut for this method later.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311216988
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java:
##########
@@ -425,7 +434,85 @@ public String branch() {
return branch;
}
- public String parquetCompressionCodec() {
+ public Map<String, String> writeProperties(FileFormat dataFormat, FileFormat deleteFormat) {
Review Comment:
Done.
--
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
[GitHub] [iceberg] szehon-ho commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317970253
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +184,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkConfOverride() {
+ List<List<Map<String, String>>> propertiesSuites =
+ Lists.newArrayList(
+ Lists.newArrayList(
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")),
+ Lists.newArrayList(
+ ImmutableMap.of(
+ COMPRESSION_CODEC,
+ "zstd",
+ SparkSQLProperties.COMPRESSION_STRATEGY,
+ "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")),
+ Lists.newArrayList(
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")));
+ for (List<Map<String, String>> propertiesSuite : propertiesSuites) {
+ testWriteProperties(propertiesSuite);
+ }
+ }
+
+ @Test
+ public void testDataPropsDefaultsAsDeleteProps() {
+ List<List<Map<String, String>>> propertiesSuites =
+ Lists.newArrayList(
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "5")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "speed")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")));
+ for (List<Map<String, String>> propertiesSuite : propertiesSuites) {
+ testWriteProperties(propertiesSuite);
+ }
+ }
+
+ @Test
+ public void testDeleteFileWriteConf() {
+ List<List<Map<String, String>>> propertiesSuites =
+ Lists.newArrayList(
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "6"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "6")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "16"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "16")));
+ for (List<Map<String, String>> propertiesSuite : propertiesSuites) {
+ testWriteProperties(propertiesSuite);
+ }
+ }
+
+ private void testWriteProperties(List<Map<String, String>> propertiesSuite) {
+ withSQLConf(
+ propertiesSuite.get(0),
+ () -> {
+ Table table = validationCatalog.loadTable(tableIdent);
+ Map<String, String> tableProperties = propertiesSuite.get(1);
+ UpdateProperties updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.set(entry.getKey(), entry.getValue());
+ }
+
+ updateProperties.commit();
+
+ Map<String, String> writeOptions = ImmutableMap.of();
+ SparkWriteConf writeConf = new SparkWriteConf(spark, table, writeOptions);
Review Comment:
Still slightly prefer to inline writeOptions here, but not a blocker.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317187559
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
Review Comment:
Fixed.
--
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
[GitHub] [iceberg] szehon-ho commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1316721395
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
+ withSQLConf(
+ (Map<String, String>) propertiesSuite[0],
+ () -> {
+ Table table = validationCatalog.loadTable(tableIdent);
+ Map<String, String> writeOptions = ImmutableMap.of();
Review Comment:
This definition seems quite far from where its used. How about just inline it below?
SparkWriteConf writeConf = new SparkWriteConf(spark, table, ImmutableMap.of());
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
is there any point to make it Object[], and not just Map<String, String>[]?
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
also, not sure 'bySuite' adds any value here, maybe just testWriteProperties
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
Review Comment:
While this is good for code re-use and test coverage , its hard to read. Iceberg test typically split to be as fine grained as possible.
I see you have at least three cases, can we make separate test for the three cases? We should also inline the properties in the test itself to make it easier to read.
```
@Test
testSparkConfOverride() {
Map<String, String>[] properties = // inline properties here
testWriteProperties(properties)...
}
testDataPropsDefaultsAsDeleteProps() {
...
}
testDeleteFileWriteConf() {
...
}
...
feel free to clarify the titles.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317197724
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
> also, not sure 'bySuite' adds any value here, maybe just testWriteProperties
Fixed.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311224014
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,245 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ Object[][] propertiesSuites =
+ new Object[][] {
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")
+ },
+ {
+ ImmutableMap.of(
+ COMPRESSION_CODEC, "zstd", SparkSQLProperties.COMPRESSION_STRATEGY, "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")
+ },
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "5")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "speed")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
Review Comment:
Case 3: Delete file properties take effect.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1312375971
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java:
##########
@@ -425,7 +431,96 @@ public String branch() {
return branch;
}
- public String parquetCompressionCodec() {
+ public Map<String, String> writeProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+ writeProperties.putAll(dataWriteProperties());
+ writeProperties.putAll(deleteWriteProperties());
+ return writeProperties;
+ }
+
+ private Map<String, String> dataWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat dataFormat = dataFileFormat();
+ switch (dataFormat) {
+ case PARQUET:
+ writeProperties.put(PARQUET_COMPRESSION, parquetCompressionCodec());
+ String parquetCompressionLevel = parquetCompressionLevel();
+ if (parquetCompressionLevel != null) {
+ writeProperties.put(PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel);
+ }
+ break;
+
+ case AVRO:
+ writeProperties.put(AVRO_COMPRESSION, avroCompressionCodec());
+ String avroCompressionLevel = avroCompressionLevel();
+ if (avroCompressionLevel != null) {
+ writeProperties.put(AVRO_COMPRESSION_LEVEL, avroCompressionLevel);
+ }
+ break;
+
+ case ORC:
+ writeProperties.put(ORC_COMPRESSION, orcCompressionCodec());
+ writeProperties.put(ORC_COMPRESSION_STRATEGY, orcCompressionStrategy());
+ break;
+
+ default:
+ // skip
+ }
+
+ return writeProperties;
+ }
+
+ private Map<String, String> deleteWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat deleteFormat = deleteFileFormat();
+ switch (deleteFormat) {
+ case PARQUET:
+ String parquetCodec = deleteParquetCompressionCodec();
+ writeProperties.put(
+ DELETE_PARQUET_COMPRESSION,
+ parquetCodec != null ? parquetCodec : parquetCompressionCodec());
+ String deleteParquetCompressionLevel = deleteParquetCompressionLevel();
+ if (deleteParquetCompressionLevel != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, deleteParquetCompressionLevel);
+ } else if (parquetCompressionLevel() != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel());
+ }
+ break;
+
+ case AVRO:
+ String avroCodec = deleteAvroCompressionCodec();
+ writeProperties.put(
+ DELETE_AVRO_COMPRESSION, avroCodec != null ? avroCodec : avroCompressionCodec());
+ String deleteAvroCompressionLevel = deleteAvroCompressionLevel();
+ if (deleteAvroCompressionLevel != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, deleteAvroCompressionLevel);
+ } else if (avroCompressionLevel() != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, avroCompressionLevel());
+ }
+ break;
+
+ case ORC:
+ String orcCodec = deleteOrcCompressionCodec();
+ writeProperties.put(
+ DELETE_ORC_COMPRESSION, orcCodec != null ? orcCodec : orcCompressionCodec());
+ String strategy = deleteOrcCompressionStrategy();
+ if (strategy != null) {
Review Comment:
Thanks. Fixed.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317190427
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
> is there any point to make it Object[], and not just Map<String, String>[]?
We can't create a generic type array. If we write
```
Map<String, String>[][] properties = new Map<String, String> [][] {
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"parquet",
DELETE_DEFAULT_FILE_FORMAT,
"parquet",
TableProperties.PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6"),
ImmutableMap.of(
DELETE_PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"orc",
DELETE_DEFAULT_FILE_FORMAT,
"orc",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION,
"zstd",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression"),
ImmutableMap.of(
DELETE_ORC_COMPRESSION,
"zstd",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"avro",
DELETE_DEFAULT_FILE_FORMAT,
"avro",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION,
"zstd",
DELETE_AVRO_COMPRESSION_LEVEL,
"16"),
ImmutableMap.of(
DELETE_AVRO_COMPRESSION,
"zstd",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION_LEVEL,
"16")
}
};
```
The compiler will report an error.
We can't cast an object array to a Map array, too.
```
Object[][] objects = new Object[][] {
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"parquet",
DELETE_DEFAULT_FILE_FORMAT,
"parquet",
TableProperties.PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6"),
ImmutableMap.of(
DELETE_PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"orc",
DELETE_DEFAULT_FILE_FORMAT,
"orc",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION,
"zstd",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression"),
ImmutableMap.of(
DELETE_ORC_COMPRESSION,
"zstd",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"avro",
DELETE_DEFAULT_FILE_FORMAT,
"avro",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION,
"zstd",
DELETE_AVRO_COMPRESSION_LEVEL,
"16"),
ImmutableMap.of(
DELETE_AVRO_COMPRESSION,
"zstd",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION_LEVEL,
"16")
}
};;
Map<String, String>[][] propertiesSuites = (Map<String, String>[][]) objects;
```
So I use Object[] here.
--
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
[GitHub] [iceberg] szehon-ho commented on pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#issuecomment-1709323529
Merged, thanks @jerqi !
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311222994
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,245 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ Object[][] propertiesSuites =
+ new Object[][] {
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")
+ },
+ {
+ ImmutableMap.of(
+ COMPRESSION_CODEC, "zstd", SparkSQLProperties.COMPRESSION_STRATEGY, "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")
+ },
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
Review Comment:
Data file properties override the delete file properties when we don't set delete file properties.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311296758
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
+ withSQLConf(
+ (Map<String, String>) propertiesSuite[0],
+ () -> {
+ Table table = validationCatalog.loadTable(tableIdent);
+ Map<String, String> writeOptions = ImmutableMap.of();
+ Map<String, String> tableProperties = (Map<String, String>) propertiesSuite[1];
+ UpdateProperties updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.set(entry.getKey(), entry.getValue());
+ }
+
+ updateProperties.commit();
+
+ SparkWriteConf writeConf = new SparkWriteConf(spark, table, writeOptions);
+ Map<String, String> writeProperties = writeConf.writeProperties();
+ Map<String, String> expectedProperties = (Map<String, String>) propertiesSuite[2];
+ Assert.assertEquals(expectedProperties.size(), writeConf.writeProperties().size());
+ for (Map.Entry<String, String> entry : writeProperties.entrySet()) {
+ Assert.assertEquals(entry.getValue(), expectedProperties.get(entry.getKey()));
+ }
+
+ table.refresh();
+ updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.remove(entry.getKey());
+ }
+
+ updateProperties.commit();
+ });
+ }
+
+ private Object[][] getOverridePropertiesSuites() {
Review Comment:
case 1: Spark properties override the write properties.
--
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
[GitHub] [iceberg] szehon-ho merged pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho merged PR #8438:
URL: https://github.com/apache/iceberg/pull/8438
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311221791
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,245 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ Object[][] propertiesSuites =
+ new Object[][] {
+ {
Review Comment:
Case 1: Spark properties override other properties.
--
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
[GitHub] [iceberg] szehon-ho commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1312318991
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java:
##########
@@ -425,7 +431,96 @@ public String branch() {
return branch;
}
- public String parquetCompressionCodec() {
+ public Map<String, String> writeProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+ writeProperties.putAll(dataWriteProperties());
+ writeProperties.putAll(deleteWriteProperties());
+ return writeProperties;
+ }
+
+ private Map<String, String> dataWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat dataFormat = dataFileFormat();
+ switch (dataFormat) {
+ case PARQUET:
+ writeProperties.put(PARQUET_COMPRESSION, parquetCompressionCodec());
+ String parquetCompressionLevel = parquetCompressionLevel();
+ if (parquetCompressionLevel != null) {
+ writeProperties.put(PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel);
+ }
+ break;
+
+ case AVRO:
+ writeProperties.put(AVRO_COMPRESSION, avroCompressionCodec());
+ String avroCompressionLevel = avroCompressionLevel();
+ if (avroCompressionLevel != null) {
+ writeProperties.put(AVRO_COMPRESSION_LEVEL, avroCompressionLevel);
+ }
+ break;
+
+ case ORC:
+ writeProperties.put(ORC_COMPRESSION, orcCompressionCodec());
+ writeProperties.put(ORC_COMPRESSION_STRATEGY, orcCompressionStrategy());
+ break;
+
+ default:
+ // skip
+ }
+
+ return writeProperties;
+ }
+
+ private Map<String, String> deleteWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat deleteFormat = deleteFileFormat();
+ switch (deleteFormat) {
+ case PARQUET:
+ String parquetCodec = deleteParquetCompressionCodec();
+ writeProperties.put(
+ DELETE_PARQUET_COMPRESSION,
+ parquetCodec != null ? parquetCodec : parquetCompressionCodec());
+ String deleteParquetCompressionLevel = deleteParquetCompressionLevel();
+ if (deleteParquetCompressionLevel != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, deleteParquetCompressionLevel);
+ } else if (parquetCompressionLevel() != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel());
+ }
+ break;
+
+ case AVRO:
+ String avroCodec = deleteAvroCompressionCodec();
+ writeProperties.put(
+ DELETE_AVRO_COMPRESSION, avroCodec != null ? avroCodec : avroCompressionCodec());
+ String deleteAvroCompressionLevel = deleteAvroCompressionLevel();
+ if (deleteAvroCompressionLevel != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, deleteAvroCompressionLevel);
+ } else if (avroCompressionLevel() != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, avroCompressionLevel());
+ }
+ break;
+
+ case ORC:
+ String orcCodec = deleteOrcCompressionCodec();
Review Comment:
Nit: would a helper method be better to re-use?
setWithFallback(writeProperties, DELETE_ORC_COMPRESSION, deleteOrcCompressionCodec(), orcCompressionCodec());
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java:
##########
@@ -425,7 +431,96 @@ public String branch() {
return branch;
}
- public String parquetCompressionCodec() {
+ public Map<String, String> writeProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+ writeProperties.putAll(dataWriteProperties());
+ writeProperties.putAll(deleteWriteProperties());
+ return writeProperties;
+ }
+
+ private Map<String, String> dataWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat dataFormat = dataFileFormat();
+ switch (dataFormat) {
+ case PARQUET:
+ writeProperties.put(PARQUET_COMPRESSION, parquetCompressionCodec());
+ String parquetCompressionLevel = parquetCompressionLevel();
+ if (parquetCompressionLevel != null) {
+ writeProperties.put(PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel);
+ }
+ break;
+
+ case AVRO:
+ writeProperties.put(AVRO_COMPRESSION, avroCompressionCodec());
+ String avroCompressionLevel = avroCompressionLevel();
+ if (avroCompressionLevel != null) {
+ writeProperties.put(AVRO_COMPRESSION_LEVEL, avroCompressionLevel);
+ }
+ break;
+
+ case ORC:
+ writeProperties.put(ORC_COMPRESSION, orcCompressionCodec());
+ writeProperties.put(ORC_COMPRESSION_STRATEGY, orcCompressionStrategy());
+ break;
+
+ default:
+ // skip
+ }
+
+ return writeProperties;
+ }
+
+ private Map<String, String> deleteWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat deleteFormat = deleteFileFormat();
+ switch (deleteFormat) {
+ case PARQUET:
+ String parquetCodec = deleteParquetCompressionCodec();
+ writeProperties.put(
+ DELETE_PARQUET_COMPRESSION,
+ parquetCodec != null ? parquetCodec : parquetCompressionCodec());
+ String deleteParquetCompressionLevel = deleteParquetCompressionLevel();
+ if (deleteParquetCompressionLevel != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, deleteParquetCompressionLevel);
+ } else if (parquetCompressionLevel() != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel());
+ }
+ break;
+
+ case AVRO:
+ String avroCodec = deleteAvroCompressionCodec();
+ writeProperties.put(
+ DELETE_AVRO_COMPRESSION, avroCodec != null ? avroCodec : avroCompressionCodec());
+ String deleteAvroCompressionLevel = deleteAvroCompressionLevel();
+ if (deleteAvroCompressionLevel != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, deleteAvroCompressionLevel);
+ } else if (avroCompressionLevel() != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, avroCompressionLevel());
+ }
+ break;
+
+ case ORC:
+ String orcCodec = deleteOrcCompressionCodec();
+ writeProperties.put(
+ DELETE_ORC_COMPRESSION, orcCodec != null ? orcCodec : orcCompressionCodec());
+ String strategy = deleteOrcCompressionStrategy();
+ if (strategy != null) {
Review Comment:
Simlarly, maybe setWithFallback(writeProperties, DELETE_ORC_COMPRESSION_STRATEGY, deleteOrcCompressionStrategy(), orcCompressionStrategy()) (I feel null handling is similar in the two cases?)
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1312375857
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/SparkWriteConf.java:
##########
@@ -425,7 +431,96 @@ public String branch() {
return branch;
}
- public String parquetCompressionCodec() {
+ public Map<String, String> writeProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+ writeProperties.putAll(dataWriteProperties());
+ writeProperties.putAll(deleteWriteProperties());
+ return writeProperties;
+ }
+
+ private Map<String, String> dataWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat dataFormat = dataFileFormat();
+ switch (dataFormat) {
+ case PARQUET:
+ writeProperties.put(PARQUET_COMPRESSION, parquetCompressionCodec());
+ String parquetCompressionLevel = parquetCompressionLevel();
+ if (parquetCompressionLevel != null) {
+ writeProperties.put(PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel);
+ }
+ break;
+
+ case AVRO:
+ writeProperties.put(AVRO_COMPRESSION, avroCompressionCodec());
+ String avroCompressionLevel = avroCompressionLevel();
+ if (avroCompressionLevel != null) {
+ writeProperties.put(AVRO_COMPRESSION_LEVEL, avroCompressionLevel);
+ }
+ break;
+
+ case ORC:
+ writeProperties.put(ORC_COMPRESSION, orcCompressionCodec());
+ writeProperties.put(ORC_COMPRESSION_STRATEGY, orcCompressionStrategy());
+ break;
+
+ default:
+ // skip
+ }
+
+ return writeProperties;
+ }
+
+ private Map<String, String> deleteWriteProperties() {
+ Map<String, String> writeProperties = Maps.newHashMap();
+
+ FileFormat deleteFormat = deleteFileFormat();
+ switch (deleteFormat) {
+ case PARQUET:
+ String parquetCodec = deleteParquetCompressionCodec();
+ writeProperties.put(
+ DELETE_PARQUET_COMPRESSION,
+ parquetCodec != null ? parquetCodec : parquetCompressionCodec());
+ String deleteParquetCompressionLevel = deleteParquetCompressionLevel();
+ if (deleteParquetCompressionLevel != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, deleteParquetCompressionLevel);
+ } else if (parquetCompressionLevel() != null) {
+ writeProperties.put(DELETE_PARQUET_COMPRESSION_LEVEL, parquetCompressionLevel());
+ }
+ break;
+
+ case AVRO:
+ String avroCodec = deleteAvroCompressionCodec();
+ writeProperties.put(
+ DELETE_AVRO_COMPRESSION, avroCodec != null ? avroCodec : avroCompressionCodec());
+ String deleteAvroCompressionLevel = deleteAvroCompressionLevel();
+ if (deleteAvroCompressionLevel != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, deleteAvroCompressionLevel);
+ } else if (avroCompressionLevel() != null) {
+ writeProperties.put(DELETE_AVRO_COMPRESSION_LEVEL, avroCompressionLevel());
+ }
+ break;
+
+ case ORC:
+ String orcCodec = deleteOrcCompressionCodec();
Review Comment:
Thanks, good point.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317157192
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
> is there any point to make it Object[]
Fixed.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311222994
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,245 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ Object[][] propertiesSuites =
+ new Object[][] {
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")
+ },
+ {
+ ImmutableMap.of(
+ COMPRESSION_CODEC, "zstd", SparkSQLProperties.COMPRESSION_STRATEGY, "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")
+ },
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
Review Comment:
case 2: Data file properties override the delete file properties when we don't set delete file properties.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311224014
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,245 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ Object[][] propertiesSuites =
+ new Object[][] {
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")
+ },
+ {
+ ImmutableMap.of(
+ COMPRESSION_CODEC, "zstd", SparkSQLProperties.COMPRESSION_STRATEGY, "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")
+ },
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "5")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "speed")
+ },
+ {
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
Review Comment:
Case 3: Delete file properties take effect.
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,245 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ Object[][] propertiesSuites =
+ new Object[][] {
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")
+ },
+ {
+ ImmutableMap.of(
+ COMPRESSION_CODEC, "zstd", SparkSQLProperties.COMPRESSION_STRATEGY, "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")
+ },
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ {
Review Comment:
case 2: Data file properties override the delete file properties when we don't set delete file properties.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311221791
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,245 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ Object[][] propertiesSuites =
+ new Object[][] {
+ {
Review Comment:
Case 1: Spark properties override other properties.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317190427
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
> is there any point to make it Object[], and not just Map<String, String>[]?
We can't create a generic type array. If we write
```
Map<String, String> properties = new Map<String, String> {xxx, xxxx};
```
The compiler will report an error.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1311298005
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
+ withSQLConf(
+ (Map<String, String>) propertiesSuite[0],
+ () -> {
+ Table table = validationCatalog.loadTable(tableIdent);
+ Map<String, String> writeOptions = ImmutableMap.of();
+ Map<String, String> tableProperties = (Map<String, String>) propertiesSuite[1];
+ UpdateProperties updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.set(entry.getKey(), entry.getValue());
+ }
+
+ updateProperties.commit();
+
+ SparkWriteConf writeConf = new SparkWriteConf(spark, table, writeOptions);
+ Map<String, String> writeProperties = writeConf.writeProperties();
+ Map<String, String> expectedProperties = (Map<String, String>) propertiesSuite[2];
+ Assert.assertEquals(expectedProperties.size(), writeConf.writeProperties().size());
+ for (Map.Entry<String, String> entry : writeProperties.entrySet()) {
+ Assert.assertEquals(entry.getValue(), expectedProperties.get(entry.getKey()));
+ }
+
+ table.refresh();
+ updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.remove(entry.getKey());
+ }
+
+ updateProperties.commit();
+ });
+ }
+
+ private Object[][] getOverridePropertiesSuites() {
+ return new Object[][] {
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")
+ },
+ {
+ ImmutableMap.of(
+ COMPRESSION_CODEC, "zstd", SparkSQLProperties.COMPRESSION_STRATEGY, "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")
+ },
+ {
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")
+ },
+ };
+ }
+
+ private Object[][] getNonOverridePropertiesSuite() {
+ return new Object[][] {
Review Comment:
case 2: We reuse the data file format properties because of the absence of delete file format properties.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317190427
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
> is there any point to make it Object[], and not just Map<String, String>[]?
We can't create a generic type array or cast the arry. So I use Object[] here. This seems confusing. So I changed the type from the Array to the List.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317190427
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +182,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkWriteConfWriteProperties() {
+ for (Object[] propertiesSuite : getOverridePropertiesSuites()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+
+ for (Object[] propertiesSuite : getNonOverridePropertiesSuite()) {
+ testWritePropertiesBySuite(propertiesSuite);
+ }
+ }
+
+ private void testWritePropertiesBySuite(Object[] propertiesSuite) {
Review Comment:
> is there any point to make it Object[], and not just Map<String, String>[]?
We can't create a generic type array. If we write
```
Map<String, String>[][] properties = new Map<String, String> [][] {
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"parquet",
DELETE_DEFAULT_FILE_FORMAT,
"parquet",
TableProperties.PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6"),
ImmutableMap.of(
DELETE_PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"orc",
DELETE_DEFAULT_FILE_FORMAT,
"orc",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION,
"zstd",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression"),
ImmutableMap.of(
DELETE_ORC_COMPRESSION,
"zstd",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"avro",
DELETE_DEFAULT_FILE_FORMAT,
"avro",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION,
"zstd",
DELETE_AVRO_COMPRESSION_LEVEL,
"16"),
ImmutableMap.of(
DELETE_AVRO_COMPRESSION,
"zstd",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION_LEVEL,
"16")
}
};
```
The compiler will report an error.
We can't cast an object array to Map array, too.
```
Object[][] objects = new Object[][] {
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"parquet",
DELETE_DEFAULT_FILE_FORMAT,
"parquet",
TableProperties.PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6"),
ImmutableMap.of(
DELETE_PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION,
"zstd",
PARQUET_COMPRESSION_LEVEL,
"5",
DELETE_PARQUET_COMPRESSION_LEVEL,
"6")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"orc",
DELETE_DEFAULT_FILE_FORMAT,
"orc",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION,
"zstd",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression"),
ImmutableMap.of(
DELETE_ORC_COMPRESSION,
"zstd",
ORC_COMPRESSION,
"snappy",
ORC_COMPRESSION_STRATEGY,
"speed",
DELETE_ORC_COMPRESSION_STRATEGY,
"compression")
},
{
ImmutableMap.of(),
ImmutableMap.of(
DEFAULT_FILE_FORMAT,
"avro",
DELETE_DEFAULT_FILE_FORMAT,
"avro",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION,
"zstd",
DELETE_AVRO_COMPRESSION_LEVEL,
"16"),
ImmutableMap.of(
DELETE_AVRO_COMPRESSION,
"zstd",
AVRO_COMPRESSION,
"snappy",
AVRO_COMPRESSION_LEVEL,
"9",
DELETE_AVRO_COMPRESSION_LEVEL,
"16")
}
};;
Map<String, String>[][] propertiesSuites = (Map<String, String>[][]) objects;
```
So I use Object[] here.
--
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
[GitHub] [iceberg] jerqi commented on a diff in pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#discussion_r1317991933
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/TestSparkWriteConf.java:
##########
@@ -164,6 +184,259 @@ public void testSparkWriteConfDistributionModeWithEverything() {
});
}
+ @Test
+ public void testSparkConfOverride() {
+ List<List<Map<String, String>>> propertiesSuites =
+ Lists.newArrayList(
+ Lists.newArrayList(
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "3"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "gzip",
+ TableProperties.DELETE_PARQUET_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "3",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "3")),
+ Lists.newArrayList(
+ ImmutableMap.of(
+ COMPRESSION_CODEC,
+ "zstd",
+ SparkSQLProperties.COMPRESSION_STRATEGY,
+ "compression"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "zlib",
+ DELETE_ORC_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression",
+ ORC_COMPRESSION_STRATEGY,
+ "compression")),
+ Lists.newArrayList(
+ ImmutableMap.of(COMPRESSION_CODEC, "zstd", COMPRESSION_LEVEL, "9"),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "gzip",
+ DELETE_AVRO_COMPRESSION,
+ "snappy"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")));
+ for (List<Map<String, String>> propertiesSuite : propertiesSuites) {
+ testWriteProperties(propertiesSuite);
+ }
+ }
+
+ @Test
+ public void testDataPropsDefaultsAsDeleteProps() {
+ List<List<Map<String, String>>> propertiesSuites =
+ Lists.newArrayList(
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "5")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "speed")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "9")));
+ for (List<Map<String, String>> propertiesSuite : propertiesSuites) {
+ testWriteProperties(propertiesSuite);
+ }
+ }
+
+ @Test
+ public void testDeleteFileWriteConf() {
+ List<List<Map<String, String>>> propertiesSuites =
+ Lists.newArrayList(
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "parquet",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "parquet",
+ TableProperties.PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "6"),
+ ImmutableMap.of(
+ DELETE_PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION,
+ "zstd",
+ PARQUET_COMPRESSION_LEVEL,
+ "5",
+ DELETE_PARQUET_COMPRESSION_LEVEL,
+ "6")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "orc",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "orc",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression"),
+ ImmutableMap.of(
+ DELETE_ORC_COMPRESSION,
+ "zstd",
+ ORC_COMPRESSION,
+ "snappy",
+ ORC_COMPRESSION_STRATEGY,
+ "speed",
+ DELETE_ORC_COMPRESSION_STRATEGY,
+ "compression")),
+ Lists.newArrayList(
+ ImmutableMap.of(),
+ ImmutableMap.of(
+ DEFAULT_FILE_FORMAT,
+ "avro",
+ DELETE_DEFAULT_FILE_FORMAT,
+ "avro",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "16"),
+ ImmutableMap.of(
+ DELETE_AVRO_COMPRESSION,
+ "zstd",
+ AVRO_COMPRESSION,
+ "snappy",
+ AVRO_COMPRESSION_LEVEL,
+ "9",
+ DELETE_AVRO_COMPRESSION_LEVEL,
+ "16")));
+ for (List<Map<String, String>> propertiesSuite : propertiesSuites) {
+ testWriteProperties(propertiesSuite);
+ }
+ }
+
+ private void testWriteProperties(List<Map<String, String>> propertiesSuite) {
+ withSQLConf(
+ propertiesSuite.get(0),
+ () -> {
+ Table table = validationCatalog.loadTable(tableIdent);
+ Map<String, String> tableProperties = propertiesSuite.get(1);
+ UpdateProperties updateProperties = table.updateProperties();
+ for (Map.Entry<String, String> entry : tableProperties.entrySet()) {
+ updateProperties.set(entry.getKey(), entry.getValue());
+ }
+
+ updateProperties.commit();
+
+ Map<String, String> writeOptions = ImmutableMap.of();
+ SparkWriteConf writeConf = new SparkWriteConf(spark, table, writeOptions);
Review Comment:
I will raise a follow-up pr to fix 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: 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
[GitHub] [iceberg] jerqi commented on pull request #8438: Spark 3.4: Fix write and SQL options to override delete file compression config
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on PR #8438:
URL: https://github.com/apache/iceberg/pull/8438#issuecomment-1709339143
> Merged, thanks @jerqi !
Thanks for your careful 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