You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nastra (via GitHub)" <gi...@apache.org> on 2023/05/11 17:14:13 UTC
[GitHub] [iceberg] nastra opened a new pull request, #7587: Spark 3.4: Fix Parquet read benchmarks
nastra opened a new pull request, #7587:
URL: https://github.com/apache/iceberg/pull/7587
fixes https://github.com/apache/iceberg/issues/7466
--
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] RussellSpitzer commented on a diff in pull request #7587: Spark 3.4: Fix Parquet read benchmarks
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191617504
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -156,6 +156,8 @@ public void readUsingSparkReader(Blackhole blackhole) throws IOException {
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
.set("spark.sql.parquet.fieldId.write.enabled", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
Review Comment:
Any hint why we need these new params?
--
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] nastra commented on a diff in pull request #7587: Spark: Fix Parquet read benchmarks for Spark 3.3 + 3.4
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191939627
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -214,6 +216,8 @@ public void readWithProjectionUsingSparkReader(Blackhole blackhole) throws IOExc
.set("spark.sql.parquet.binaryAsString", "false")
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
+ .set("spark.sql.legacy.parquet.nanosAsLong", "false")
Review Comment:
I've included a fix for the Spark 3.3 benchmarks in 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
[GitHub] [iceberg] aokolnychyi commented on a diff in pull request #7587: Spark 3.4: Fix Parquet read benchmarks
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191663197
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -214,6 +216,8 @@ public void readWithProjectionUsingSparkReader(Blackhole blackhole) throws IOExc
.set("spark.sql.parquet.binaryAsString", "false")
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
+ .set("spark.sql.legacy.parquet.nanosAsLong", "false")
Review Comment:
Why do we need the second one? It seems it was added in 3.2.
--
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] nastra commented on a diff in pull request #7587: Spark 3.4: Fix Parquet read benchmarks
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191934971
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -214,6 +216,8 @@ public void readWithProjectionUsingSparkReader(Blackhole blackhole) throws IOExc
.set("spark.sql.parquet.binaryAsString", "false")
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
+ .set("spark.sql.legacy.parquet.nanosAsLong", "false")
Review Comment:
it seems that `spark.sql.legacy.parquet.nanosAsLong` might be required for our Spark [3.3](https://github.com/apache/spark/blame/v3.3.2/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaConverter.scala#L70) benchmarks as well. Our 3.2 benchmarks aren't affected because we're on 3.2.2 and that flag was added to 3.2.4.
--
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] nastra commented on a diff in pull request #7587: Spark 3.4: Fix Parquet read benchmarks
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191931833
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -156,6 +156,8 @@ public void readUsingSparkReader(Blackhole blackhole) throws IOException {
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
.set("spark.sql.parquet.fieldId.write.enabled", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
Review Comment:
This is due to these 2 flags being required: https://github.com/apache/spark/blame/branch-3.4/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaConverter.scala#L72-L73
--
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] aokolnychyi commented on a diff in pull request #7587: Spark 3.4: Fix Parquet read benchmarks
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191662608
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -156,6 +156,8 @@ public void readUsingSparkReader(Blackhole blackhole) throws IOException {
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
.set("spark.sql.parquet.fieldId.write.enabled", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
Review Comment:
I guess we are reading a file not written by Spark so there is no Spark schema written. That's why Spark tries to infer the actual timestamp type.
--
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] nastra commented on pull request #7587: Spark: Fix Parquet read benchmarks for Spark 3.3 + 3.4
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#issuecomment-1547851450
thanks for the reviews @RussellSpitzer and @aokolnychyi
--
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] nastra commented on a diff in pull request #7587: Spark 3.4: Fix Parquet read benchmarks
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191931666
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -214,6 +216,8 @@ public void readWithProjectionUsingSparkReader(Blackhole blackhole) throws IOExc
.set("spark.sql.parquet.binaryAsString", "false")
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
+ .set("spark.sql.legacy.parquet.nanosAsLong", "false")
Review Comment:
both flags were just recently added to https://github.com/apache/spark/blame/branch-3.4/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaConverter.scala#L72-L73
--
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] nastra commented on a diff in pull request #7587: Spark 3.4: Fix Parquet read benchmarks
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7587:
URL: https://github.com/apache/iceberg/pull/7587#discussion_r1191934971
##########
spark/v3.4/spark/src/jmh/java/org/apache/iceberg/spark/data/parquet/SparkParquetReadersFlatDataBenchmark.java:
##########
@@ -214,6 +216,8 @@ public void readWithProjectionUsingSparkReader(Blackhole blackhole) throws IOExc
.set("spark.sql.parquet.binaryAsString", "false")
.set("spark.sql.parquet.int96AsTimestamp", "false")
.set("spark.sql.caseSensitive", "false")
+ .set("spark.sql.parquet.inferTimestampNTZ.enabled", "false")
+ .set("spark.sql.legacy.parquet.nanosAsLong", "false")
Review Comment:
it seems that `spark.sql.legacy.parquet.nanosAsLong` might be required for our Spark [3.3](https://github.com/apache/spark/blame/v3.2.4/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaConverter.scala) + [3.2](https://github.com/apache/spark/blame/v3.3.2/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaConverter.scala#L70) benchmarks as well
--
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] nastra merged pull request #7587: Spark: Fix Parquet read benchmarks for Spark 3.3 + 3.4
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #7587:
URL: https://github.com/apache/iceberg/pull/7587
--
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