Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MINOR]: Remove unused Results #8189

Merged
merged 1 commit into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions datafusion/physical-plan/src/aggregates/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,19 +405,19 @@ fn get_aggregate_search_mode(
aggr_expr: &mut [Arc<dyn AggregateExpr>],
order_by_expr: &mut [Option<LexOrdering>],
ordering_req: &mut Vec<PhysicalSortExpr>,
) -> Result<PartitionSearchMode> {
) -> PartitionSearchMode {
let groupby_exprs = group_by
.expr
.iter()
.map(|(item, _)| item.clone())
.collect::<Vec<_>>();
let mut partition_search_mode = PartitionSearchMode::Linear;
if !group_by.is_single() || groupby_exprs.is_empty() {
return Ok(partition_search_mode);
return partition_search_mode;
}

if let Some((should_reverse, mode)) =
get_window_mode(&groupby_exprs, ordering_req, input)?
get_window_mode(&groupby_exprs, ordering_req, input)
{
let all_reversible = aggr_expr
.iter()
Expand All @@ -437,7 +437,7 @@ fn get_aggregate_search_mode(
}
partition_search_mode = mode;
}
Ok(partition_search_mode)
partition_search_mode
}

/// Check whether group by expression contains all of the expression inside `requirement`
Expand Down Expand Up @@ -513,7 +513,7 @@ impl AggregateExec {
&mut aggr_expr,
&mut order_by_expr,
&mut ordering_req,
)?;
);

// Get GROUP BY expressions:
let groupby_exprs = group_by.input_exprs();
Expand Down
14 changes: 7 additions & 7 deletions datafusion/physical-plan/src/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ pub fn get_best_fitting_window(
let orderby_keys = window_exprs[0].order_by();
let (should_reverse, partition_search_mode) =
if let Some((should_reverse, partition_search_mode)) =
get_window_mode(partitionby_exprs, orderby_keys, input)?
get_window_mode(partitionby_exprs, orderby_keys, input)
{
(should_reverse, partition_search_mode)
} else {
Expand Down Expand Up @@ -467,12 +467,12 @@ pub fn get_best_fitting_window(
/// can run with existing input ordering, so we can remove `SortExec` before it.
/// The `bool` field in the return value represents whether we should reverse window
/// operator to remove `SortExec` before it. The `PartitionSearchMode` field represents
/// the mode this window operator should work in to accomodate the existing ordering.
/// the mode this window operator should work in to accommodate the existing ordering.
pub fn get_window_mode(
partitionby_exprs: &[Arc<dyn PhysicalExpr>],
orderby_keys: &[PhysicalSortExpr],
input: &Arc<dyn ExecutionPlan>,
) -> Result<Option<(bool, PartitionSearchMode)>> {
) -> Option<(bool, PartitionSearchMode)> {
let input_eqs = input.equivalence_properties();
let mut partition_by_reqs: Vec<PhysicalSortRequirement> = vec![];
let (_, indices) = input_eqs.find_longest_permutation(partitionby_exprs);
Expand All @@ -499,10 +499,10 @@ pub fn get_window_mode(
} else {
PartitionSearchMode::PartiallySorted(indices)
};
return Ok(Some((should_swap, mode)));
return Some((should_swap, mode));
}
}
Ok(None)
None
}

#[cfg(test)]
Expand Down Expand Up @@ -869,7 +869,7 @@ mod tests {
order_by_exprs.push(PhysicalSortExpr { expr, options });
}
let res =
get_window_mode(&partition_by_exprs, &order_by_exprs, &exec_unbounded)?;
get_window_mode(&partition_by_exprs, &order_by_exprs, &exec_unbounded);
// Since reversibility is not important in this test. Convert Option<(bool, PartitionSearchMode)> to Option<PartitionSearchMode>
let res = res.map(|(_, mode)| mode);
assert_eq!(
Expand Down Expand Up @@ -1033,7 +1033,7 @@ mod tests {
}

assert_eq!(
get_window_mode(&partition_by_exprs, &order_by_exprs, &exec_unbounded)?,
get_window_mode(&partition_by_exprs, &order_by_exprs, &exec_unbounded),
*expected,
"Unexpected result for in unbounded test case#: {case_idx:?}, case: {test_case:?}"
);
Expand Down