alamb commented on code in PR #6619:
URL: https://github.com/apache/arrow-datafusion/pull/6619#discussion_r1224709658
##########
datafusion/core/src/physical_plan/windows/mod.rs:
##########
@@ -245,32 +244,14 @@ pub(crate) fn window_ordering_equivalence(
.with_equivalences(input.equivalence_properties())
.with_existing_ordering(input.output_ordering().map(|elem|
elem.to_vec()))
.extend(input.ordering_equivalence_properties());
+
for expr in window_expr {
if let Some(builtin_window_expr) =
expr.as_any().downcast_ref::<BuiltInWindowExpr>()
{
- // Only the built-in `RowNumber` window function introduces a new
- // ordering:
- if builtin_window_expr
+ builtin_window_expr
.get_built_in_func_expr()
- .as_any()
- .is::<RowNumber>()
Review Comment:
The point of the PR is to remove this special case for RowNumber (which I
did by hoisting it into the trait)
##########
datafusion/physical-expr/src/window/built_in_window_function_expr.rs:
##########
@@ -34,10 +35,6 @@ use std::sync::Arc;
/// `nth_value` need the value.
#[allow(rustdoc::private_intra_doc_links)]
pub trait BuiltInWindowFunctionExpr: Send + Sync + std::fmt::Debug {
- /// Returns the aggregate expression as [`Any`](std::any::Any) so that it
can be
- /// downcast to a specific implementation.
- fn as_any(&self) -> &dyn Any;
Review Comment:
If reviewers would prefer I keep this in I can do so -- the reason I think
it would be best to avoid is so that other parts of the code can't special case
the built in window functions
--
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]
