Conversation
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The equal-fragment assumption is replaced with live, deletion-aware fragment row counts from the same pinned scan plan, while conservative fallbacks remain for empty, non-selective, and inconsistent inputs. Unequal-fragment end-to-end planning and the supported Scala/Spark variants behave as intended; I found no correctness or compatibility blocker.
lance-gatekeeper
Bot
added
the
K-approved
| @@ -67,6 +67,30 @@ public static LanceStatistics estimatePostPruning( | |||
| return new LanceStatistics((long) (totalRows * ratio), (long) (totalFilesSize * ratio)); | |||
There was a problem hiding this comment.
oh interesting, we arent using the estimatePostPruning and is dead code by the looks of it. Maybe we should remove this in a follow up or somehting.
| return new LanceStatistics((long) (totalRows * ratio), (long) (totalFilesSize * ratio)); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
nit: this javadoc is pretty verbose we can probably shorten if not drop the java doc wdyt?
| // Zonemap stats can still contain retired fragment IDs. Only row counts from the live | ||
| // fragments in this scan plan may contribute to the estimate. |
There was a problem hiding this comment.
nit: getOrDefault(fragmentId, 0L) is already droppping retired ids. the comment on top seems redundant maybe we can drop or simplify?
2 participants
Summary
Testing
./mvnw test -pl lance-spark-3.5_2.13 -am -Dtest=LanceStatisticsTest,LanceSplitTest,LanceScanTest,LanceScanBuilderTest -Dsurefire.failIfNoSpecifiedTests=false./mvnw spotless:check -pl lance-spark-base_2.12./mvnw test -pl lance-spark-3.5_2.12 -am -Dtest=LanceStatisticsTest -Dsurefire.failIfNoSpecifiedTests=false