alamb commented on code in PR #25767:
URL: https://github.com/apache/datafusion/pull/25767#discussion_r4366855379
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -69,6 +69,10 @@ use datafusion_expr::simplify::SimplifyContext;
use datafusion_expr::{
AggregateUDF, Explain, Expr, HigherOrderUDF, LogicalPlan, ScalarUDF,
WindowUDF,
};
+#[cfg(feature = "nested_expressions")]
+use datafusion_functions_aggregate::count::count_udaf;
Review Comment:
I think this is a bad idea -- we should not have the session state depending
on aggregate functions
##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -1497,6 +1512,198 @@ impl LogicalPlanBuilder {
)
}
+ /// Build an `INTERSECT ALL` plan, preserving the multiplicity of each row.
+ ///
+ /// Each distinct row is returned `min(m, n)` times, where `m` and `n` are
+ /// its counts on the left and right. `count` must be the `count` aggregate
+ /// function and `range` the `range` scalar function: rows are counted on
+ /// each side and then expanded back with `unnest(range(..))`.
+ ///
+ /// Rows are compared in the same type that `=` compares them in. If the
+ /// type of a column differs between the inputs, both are cast to that type
+ /// before the rows are counted, and the result has that type instead of
the
+ /// type of the left input. Values that are equal after the cast, such as
the
+ /// strings `'1'` and `'01'` and the integer `1`, count as the same row.
+ pub fn intersect_all(
+ left_plan: LogicalPlan,
+ right_plan: LogicalPlan,
+ count: &Arc<AggregateUDF>,
+ range: &Arc<ScalarUDF>,
+ ) -> Result<LogicalPlan> {
+ Self::multiset_set_operation(left_plan, right_plan, count, range,
JoinType::Inner)
+ }
+
+ /// Build an `EXCEPT ALL` plan, subtracting matching row multiplicities.
+ ///
+ /// Each distinct row is returned `m - n` times when `m > n`. `count` and
+ /// `range` are as for [`Self::intersect_all`], and so is the handling of
+ /// inputs whose column types differ.
+ pub fn except_all(
+ left_plan: LogicalPlan,
+ right_plan: LogicalPlan,
+ count: &Arc<AggregateUDF>,
+ range: &Arc<ScalarUDF>,
+ ) -> Result<LogicalPlan> {
+ Self::multiset_set_operation(left_plan, right_plan, count, range,
JoinType::Left)
+ }
+
+ fn multiset_set_operation(
+ left_plan: LogicalPlan,
+ right_plan: LogicalPlan,
+ count: &Arc<AggregateUDF>,
+ range: &Arc<ScalarUDF>,
+ join_type: JoinType,
+ ) -> Result<LogicalPlan> {
+ let left_len = left_plan.schema().fields().len();
+ let right_len = right_plan.schema().fields().len();
+ if left_len != right_len {
+ return plan_err!(
+ "INTERSECT/EXCEPT query must have the same number of columns.
Left is {left_len} and right is {right_len}."
+ );
+ }
+ let from_right = left_plan
Review Comment:
I think this the wrong place to be doingg this optimization -- the logical
plan builder should just make the LogicalPlan for Inersect/Except
If you want to rewrite this to a more optimized plan, then that should be
done in as an Optimizer, not when the plan is consturcted
among other problems, if you do the rewrite here, users who create their own
plans directly or otehr optimizer passes that may put in intersection/intersect
will not get the benefit of this PR
--
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]
