You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/03/31 06:10:00 UTC
[GitHub] [iceberg] singhpk234 opened a new pull request #4446: Spark [3.2] : Improve stats estimation for spark scan
singhpk234 opened a new pull request #4446:
URL: https://github.com/apache/iceberg/pull/4446
As per my understanding, presently we don't take `SplitScanTask` which is FileScanTask, into consideration where we would not be reading the complete file rather a subpart of the file so in this case it's better to find number of records by lenght() ,
rather than going to datafile this splitscanTask is based on and get's its record count.
---
cc @rdblue @aokolnychyi @RussellSpitzer @wypoon
--
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] rdblue commented on a change in pull request #4446: Spark [3.2] : Improve stats estimation for spark scan
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #4446:
URL: https://github.com/apache/iceberg/pull/4446#discussion_r839973586
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkScan.java
##########
@@ -148,7 +148,9 @@ protected Statistics estimateStatistics(Snapshot snapshot) {
for (CombinedScanTask task : tasks()) {
for (FileScanTask file : task.files()) {
- numRows += file.file().recordCount();
+ // TODO: if possible, take deletes also into consideration.
+ double fractionOfFileScanned = ((double) file.length() / file.file().fileSizeInBytes());
Review comment:
I think the parens make more sense around `(double) file.length()`. Otherwise it is still not clear whether integer division or the cast is happening first.
--
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] rdblue commented on a change in pull request #4446: Spark [3.2] : Improve stats estimation for spark scan
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #4446:
URL: https://github.com/apache/iceberg/pull/4446#discussion_r839927551
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkScan.java
##########
@@ -145,10 +145,12 @@ protected Statistics estimateStatistics(Snapshot snapshot) {
}
long numRows = 0L;
+ long defaultRowSizeInBytes = readSchema().defaultSize();
for (CombinedScanTask task : tasks()) {
for (FileScanTask file : task.files()) {
- numRows += file.file().recordCount();
+ // TODO: if possible, take deletes also into consideration.
+ numRows += Math.min(file.length() / defaultRowSizeInBytes, file.file().recordCount());
Review comment:
We always want to base the estimate on the number of records in the file and not on the file length. Calculations based on the file length are quite bad in practice because Parquet compression varies.
If you want to find out how many records from the file will actually be scanned by this task, then you could do something different, like finding the proportion of the file that is covered by the task and multiplying by the record count.
--
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] singhpk234 commented on a change in pull request #4446: Spark [3.2] : Improve stats estimation for spark scan
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on a change in pull request #4446:
URL: https://github.com/apache/iceberg/pull/4446#discussion_r839942774
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkScan.java
##########
@@ -145,10 +145,12 @@ protected Statistics estimateStatistics(Snapshot snapshot) {
}
long numRows = 0L;
+ long defaultRowSizeInBytes = readSchema().defaultSize();
for (CombinedScanTask task : tasks()) {
for (FileScanTask file : task.files()) {
- numRows += file.file().recordCount();
+ // TODO: if possible, take deletes also into consideration.
+ numRows += Math.min(file.length() / defaultRowSizeInBytes, file.file().recordCount());
Review comment:
made the changes, thanks @rdblue for the suggestion, this makes perfect sense :) !!!
--
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