hassaanch23 commented on code in PR #25654:
URL: https://github.com/apache/datafusion/pull/25654#discussion_r4152926338
##########
datafusion/physical-plan/src/statistics.rs:
##########
@@ -102,6 +102,39 @@ impl StatisticsArgs {
}
}
+/// Applies `fetch` to the `input` statistics of an operator that stops *each*
+/// of its `n_partitions` output partitions after `fetch` rows, such as
+/// `LocalLimitExec` or a `SortExec` with a fetch that preserves partitioning.
+///
+/// `input` must cover what `args` asks for: one partition's rows when `args`
+/// names a partition, and the rows of every partition otherwise.
+///
+/// For a single partition this is [`Statistics::with_fetch`]. Overall, though,
+/// every partition can emit up to `fetch` rows, so the output can reach
+/// `fetch * n_partitions` rows, and never more than the input. How the input
+/// rows are spread over the partitions is unknown, so the result is inexact
+/// unless `fetch` cannot drop any row.
+pub(crate) fn with_per_partition_fetch(
+ input: Statistics,
+ fetch: Option<usize>,
+ n_partitions: usize,
+ args: &StatisticsArgs,
+) -> Result<Statistics> {
+ let Some(fetch) = fetch else {
+ return Ok(input);
+ };
+ if args.partition().is_some() || n_partitions <= 1 {
+ return input.with_fetch(Some(fetch), 0, 1);
+ }
+ // No partition can hold more than `fetch` rows, so none is dropped.
+ if matches!(input.num_rows.get_value(), Some(&num_rows) if num_rows <=
fetch) {
Review Comment:
Good catch, thanks. Fixed in ad59f94: the early return now requires
`Precision::Exact(n)` with `n <= fetch`. An estimate goes on through
`with_fetch(fetch * n_partitions)` and `.to_inexact()`, so its column
statistics become inexact too.
I added `per_partition_fetch_keeps_exactness_only_for_exact_counts`, which
uses `Inexact(8)` with an exact null count, as you suggested. Before the change
it failed with the null count still `Exact(3)`.
The same reasoning applies to `Statistics::with_fetch` itself for a single
partition: when `nr <= fetch` it returns the input unchanged, inexact `nr`
included. That predates this PR and affects every operator with a fetch, so
I've left it out of this one. Happy to open a follow-up issue if you think it's
worth changing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
