avantgardnerio commented on code in PR #19642:
URL: https://github.com/apache/datafusion/pull/19642#discussion_r2662077807
##########
datafusion/expr/src/udwf.rs:
##########
@@ -427,21 +440,130 @@ pub trait WindowUDFImpl: Debug + DynEq + DynHash + Send
+ Sync {
None
}
- /// If not causal, returns the effect this function will have on the window
+ /// Returns the effect this function has on limit pushdowns. See
[`LimitEffect`]
+ /// for more details.
+ ///
+ /// Defaults to [`LimitEffect::Unknown`].
fn limit_effect(&self, _args: &[Arc<dyn PhysicalExpr>]) -> LimitEffect {
LimitEffect::Unknown
}
}
-/// the effect this function will have on the limit pushdown
+/// The effect this function will have on limit pushdowns through a window
bound.
Review Comment:
The goal was to move away from boolean APIs for specific optimizer rules,
i.e. `support_filter_pushdown() -> bool` because that means each UDAF needs to
know about each optimizer rule.
Instead the idea was to move towards natural mathematical properties such as
`cardinality_effect() -> CustomEnum`. This does not specifically relate to an
optimizer rule, it merely exposes information about what the LogicalPlanNode /
UDAF / etc does.
By separating these concerns, we no longer need to update everything each
time we invent a new optimizer rule (in fact, the `window_push_past_limit` rule
was able to use the pre-existing `cardinality_effect()` from a previous
optimizer rule).
This is a long-winded way to say: I think the update to this documentation
is incorrect in that it is falsely overly specific.
I'm sure the world would go on if we merged as it, but I think it helps
understanding to leave it correctly vague.
--
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]
