2010YOUY01 commented on code in PR #26069:
URL: https://github.com/apache/datafusion/pull/26069#discussion_r4204232052
##########
datafusion/physical-optimizer/src/limited_distinct_aggregation.rs:
##########
@@ -15,26 +15,68 @@
// specific language governing permissions and limitations
// under the License.
-//! A special-case optimizer rule that pushes limit into a grouped aggregation
-//! which has no aggregate expressions or sorting requirements
+//! Stop unordered DISTINCT aggregation once enough groups have been found.
use std::sync::Arc;
-use datafusion_physical_plan::aggregates::AggregateExec;
+use datafusion_physical_plan::aggregates::{AggregateExec, AggregateMode};
use datafusion_physical_plan::limit::{GlobalLimitExec, LocalLimitExec};
-use datafusion_physical_plan::{ExecutionPlan, ExecutionPlanProperties};
+use datafusion_physical_plan::{
+ ChildrenPropertiesMode, ExecutionPlan, ExecutionPlanProperties,
+ ReplaceChildrenOptions,
+};
use datafusion_common::Result;
use datafusion_common::config::ConfigOptions;
use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode};
use crate::PhysicalOptimizerRule;
-use itertools::Itertools;
-/// An optimizer rule that passes a `limit` hint into grouped aggregations
which don't require all
-/// rows in the group to be processed for correctness. Example queries fitting
this description are:
-/// - `SELECT distinct l_orderkey FROM lineitem LIMIT 10;`
-/// - `SELECT l_orderkey FROM lineitem GROUP BY l_orderkey LIMIT 10;`
+/// Pushes a soft limit into unordered DISTINCT aggregations. For
+/// `SELECT DISTINCT a FROM t LIMIT 10`, any ten distinct values are
sufficient:
+/// later rows cannot change the groups already found. The same reasoning
applies
+/// to `GROUP BY a` without aggregate expressions. Aggregates such as `SUM`
need
+/// every row in each group.
+///
+/// Each partial stage can also stop after finding ten distinct keys: those
keys
+/// remain distinct in the final stage, even if other partitions contribute
+/// duplicates.
+///
+/// Before:
+///
+/// ```txt
+/// Limit(10)
+/// Aggregate(Final, a)
+/// Aggregate(Partial, a)
+/// Scan
+/// ```
+///
+/// After:
+///
+/// ```txt
+/// Limit(10)
+/// Aggregate(Final, a, soft_limit=10)
+/// Aggregate(Partial, a, soft_limit=10)
+/// Scan
+/// ```
+///
+/// # Invariants before and after the rewrite
Review Comment:
Indeed, 'invariant' is not precise here.
Here is the updated version in
[c8986fd](https://github.com/apache/datafusion/pull/26069/commits/c8986fdd80fc65a677f66c242917ec3b1112b30a)
It should describe what it is today, but I think we could make this kind of
assumption/promise description part of a template, require it as a comment
section for all optimizer rules, and try to keep it maintained.
These assumptions are already there, but many of them are undocumented. I’m
not sure exactly how we should do this yet, so I’ll keep thinking about it in
the background.
```txt
/// # What this rule assumes
///
/// This rule assumes the logical aggregate only have one shape showed
below, this
/// is what the current physical planning produces.
///
/// ```txt
/// Limit
/// AggregateExec(mode=Final)
/// AggregateExec(mode=Partial)
/// ```
///
/// If future changes or extensions produce a different shape, this rule
skips the
/// rewrite rather than reporting an error, potentially missing an
optimization
/// opportunity.
///
/// # What this rule promises
///
/// Immediately after an eligible rewrite, both stages have a soft-limit
hint.
/// Otherwise, this rule leaves both stages unchanged.
///
/// If a later rule removes the limit, it won't affect correctness, but it
may
/// miss an optimization opportunity.
```
--
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]
