Spark 3.4 : Use correct statistics file in SparkScan::estimateStatistics(Snapshot)

[jeesou]

No description provided.

[jeesou]

This is the backport changes from #12482

@wypoon @findepi

Kindly help review and merge

[wypoon]

This is a clean backport of my fix.
@jeesou can you please use the same title as the original fix, prefixed by "Spark 3.4" --
"Spark 3.4: Use correct statistics file in SparkScan::estimateStatistics(Snapshot)".
Thanks.
(Btw, it is customary to acknowledge the author of the original fix if you're doing a backport.)

[jeesou]

Yes thanks @wypoon for this fix, and for reviewing it.

[pvary]

Could we have multiple files with the same snapshotId? Or findAny is enough?

[ajantha-bhat]

As per java implementation, it is always a single stats file per snapshot id.

iceberg/core/src/main/java/org/apache/iceberg/TableMetadata.java

Line 1369 in 695374d

statisticsFiles.put(statisticsFile.snapshotId(), ImmutableList.of(statisticsFile));

I followed similar design while working on partition stats too.

[pvary]

So we could have a theoretical performance boost from findAny

[wypoon]

The spec doesn't actually say that there should only be one statistics file per snapshot. This happens to be how it is implemented in Java. The spec simply allows for multiple statistics files.
I was thinking about the problem of tracking orphaned statistics files when they are recomputed. One idea I had was to keep replaced statistics files (for a snapshot) still in the list (as long as the files are tracked in metadata we can clean up unused ones), but to keep the newest one before others. Hence findFirst. It was just an idea (and honestly not one I'm seriously considering).
In any case, I do not think that findAny is faster than findFirst here.

[pvary]

Why is this PR against Spark 3.4?
In Flink we usually create PRs against the current version

[ajantha-bhat]

@pvary: This PR is looks like just a backport of #12482.

He has not updated the PR descriptions properly (looks like new contributor).

[pvary]

Thanks @ajantha-bhat for catching that this is a backport!

@jeesou: Please when backporting tell it in the PR description. Also highlight any changes needed compared to the original PR, so the reviewers could have easier time.

[wypoon]

@pvary, @ajantha-bhat I already reviewed this a day before you and if you read my comment, I mentioned that this is a clean backport of my fix.

@jeesou as Peter stated, you should state in the description that this is a backport of #12482 (and that I am the author of the fix). I implied it but did not state it explicitly.