[GitHub] [spark] huaxingao commented on a diff in pull request #40495: test for reading footer within file range

[via GitHub]

huaxingao commented on code in PR #40495:
URL: https://github.com/apache/spark/pull/40495#discussion_r1142903084


##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/parquet/ParquetPartitionReaderFactory.scala:
##
@@ -92,8 +93,13 @@ case class ParquetPartitionReaderFactory(
 if (aggregation.isEmpty) {
   ParquetFooterReader.readFooter(conf, filePath, SKIP_ROW_GROUPS)
 } else {
+  val split = new FileSplit(file.toPath, file.start, file.length, 
Array.empty[String])
   // For aggregate push down, we will get max/min/count from footer 
statistics.
-  ParquetFooterReader.readFooter(conf, filePath, NO_FILTER)

Review Comment:
   @sunchao What do you think?



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] [spark] huaxingao commented on a diff in pull request #40495: test for reading footer within file range

[via GitHub]

huaxingao commented on code in PR #40495:
URL: https://github.com/apache/spark/pull/40495#discussion_r1142902856


##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/parquet/ParquetPartitionReaderFactory.scala:
##
@@ -92,8 +93,13 @@ case class ParquetPartitionReaderFactory(
 if (aggregation.isEmpty) {
   ParquetFooterReader.readFooter(conf, filePath, SKIP_ROW_GROUPS)
 } else {
+  val split = new FileSplit(file.toPath, file.start, file.length, 
Array.empty[String])
   // For aggregate push down, we will get max/min/count from footer 
statistics.
-  ParquetFooterReader.readFooter(conf, filePath, NO_FILTER)

Review Comment:
   @yabola Thanks for the ping. I was assuming that we always need to read the 
whole `PartitionedFile`, i.e. the start is always 0 and the length is always 
the file length. Maybe it's safer to add the range.



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org