Skip to content

Commit 150a9ed

Browse files
orlporlp
authored
refactor(rust): Remove duplicate maintain_order from CrossJoinOptions (#24725)
1 parent 24b23d0 commit 150a9ed

File tree

7 files changed

+20
-46
lines changed

7 files changed

+20
-46
lines changed

‎crates/polars-ops/src/frame/join/args.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ where
169169
#[derive(Clone)]
170170
pub struct CrossJoinOptions {
171171
pub predicate: Arc<dyn CrossJoinFilter>,
172-
pub maintain_order: MaintainOrderJoin,
173172
}
174173

175174
impl CrossJoinOptions {
@@ -183,14 +182,12 @@ impl Eq for CrossJoinOptions {}
183182
impl PartialEq for CrossJoinOptions {
184183
fn eq(&self, other: &Self) -> bool {
185184
std::ptr::addr_eq(self.as_ptr_ref(), other.as_ptr_ref())
186-
&& self.maintain_order == other.maintain_order
187185
}
188186
}
189187

190188
impl Hash for CrossJoinOptions {
191189
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
192190
self.as_ptr_ref().hash(state);
193-
self.maintain_order.hash(state);
194191
}
195192
}
196193

‎crates/polars-ops/src/frame/join/cross_join.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,12 +138,13 @@ pub(super) fn fused_cross_filter(
138138
right: &DataFrame,
139139
suffix: Option<PlSmallStr>,
140140
cross_join_options: &CrossJoinOptions,
141+
maintain_order: MaintainOrderJoin,
141142
) -> PolarsResult<DataFrame> {
142143
let unfiltered_size = (left.height() as u64).saturating_mul(right.height() as u64);
143144
let chunk_size = (unfiltered_size / _set_partition_size() as u64).clamp(1, 100_000);
144145
let num_chunks = (unfiltered_size / chunk_size).max(1) as usize;
145146

146-
let left_is_primary = match cross_join_options.maintain_order {
147+
let left_is_primary = match maintain_order {
147148
MaintainOrderJoin::None => true,
148149
MaintainOrderJoin::Left | MaintainOrderJoin::LeftRight => true,
149150
MaintainOrderJoin::Right | MaintainOrderJoin::RightLeft => false,
@@ -165,8 +166,7 @@ pub(super) fn fused_cross_filter(
165166
let dfs = POOL
166167
.install(|| {
167168
cartesian_prod.par_iter().map(|(left, right)| {
168-
let (mut left, right) =
169-
cross_join_dfs(left, right, None, false, cross_join_options.maintain_order)?;
169+
let (mut left, right) = cross_join_dfs(left, right, None, false, maintain_order)?;
170170
let mut right_columns = right.take_columns();
171171

172172
for (c, name) in right_columns.iter_mut().zip(rename_names) {

‎crates/polars-ops/src/frame/join/mod.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,13 @@ pub trait DataFrameJoinOps: IntoDf {
134134
if let JoinType::Cross = args.how {
135135
if let Some(JoinTypeOptions::Cross(cross_options)) = &options {
136136
assert!(args.slice.is_none());
137-
return fused_cross_filter(left_df, other, args.suffix.clone(), cross_options);
137+
return fused_cross_filter(
138+
left_df,
139+
other,
140+
args.suffix.clone(),
141+
cross_options,
142+
args.maintain_order,
143+
);
138144
}
139145
return left_df.cross_join(other, args.suffix.clone(), args.slice, args.maintain_order);
140146
}

‎crates/polars-plan/src/dsl/options/mod.rs‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use polars_io::json::JsonWriterOptions;
1818
use polars_io::parquet::write::ParquetWriteOptions;
1919
#[cfg(feature = "iejoin")]
2020
use polars_ops::frame::IEJoinOptions;
21-
use polars_ops::frame::{CrossJoinFilter, CrossJoinOptions, JoinTypeOptions, MaintainOrderJoin};
21+
use polars_ops::frame::{CrossJoinFilter, CrossJoinOptions, JoinTypeOptions};
2222
use polars_ops::prelude::{JoinArgs, JoinType};
2323
#[cfg(feature = "dynamic_group_by")]
2424
use polars_time::DynamicGroupOptions;
@@ -78,7 +78,6 @@ pub enum JoinTypeOptionsIR {
7878
// Fused cross join and filter (only used in the in-memory engine)
7979
CrossAndFilter {
8080
predicate: ExprIR, // Must be elementwise.
81-
maintain_order: MaintainOrderJoin,
8281
},
8382
}
8483

@@ -88,12 +87,8 @@ impl Hash for JoinTypeOptionsIR {
8887
match self {
8988
#[cfg(feature = "iejoin")]
9089
IEJoin(opt) => opt.hash(state),
91-
CrossAndFilter {
92-
predicate,
93-
maintain_order,
94-
} => {
90+
CrossAndFilter { predicate } => {
9591
predicate.node().hash(state);
96-
maintain_order.hash(state);
9792
},
9893
}
9994
}
@@ -106,16 +101,10 @@ impl JoinTypeOptionsIR {
106101
) -> PolarsResult<JoinTypeOptions> {
107102
use JoinTypeOptionsIR::*;
108103
match self {
109-
CrossAndFilter {
110-
predicate,
111-
maintain_order,
112-
} => {
104+
CrossAndFilter { predicate } => {
113105
let predicate = plan(&predicate)?;
114106

115-
Ok(JoinTypeOptions::Cross(CrossJoinOptions {
116-
predicate,
117-
maintain_order,
118-
}))
107+
Ok(JoinTypeOptions::Cross(CrossJoinOptions { predicate }))
119108
},
120109
#[cfg(feature = "iejoin")]
121110
IEJoin(opt) => Ok(JoinTypeOptions::IEJoin(opt)),

‎crates/polars-plan/src/plans/ir/format.rs‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -200,11 +200,7 @@ impl<'a> IRDisplay<'a> {
200200
let right_on = self.display_expr_slice(right_on);
201201

202202
// Fused cross + filter (show as nested loop join)
203-
if let Some(JoinTypeOptionsIR::CrossAndFilter {
204-
predicate,
205-
maintain_order: _,
206-
}) = &options.options
207-
{
203+
if let Some(JoinTypeOptionsIR::CrossAndFilter { predicate }) = &options.options {
208204
let predicate = self.display_expr(predicate);
209205
let name = "NESTED LOOP";
210206
write!(f, "{:indent$}{name} JOIN ON {predicate}:", "")?;
@@ -881,11 +877,7 @@ pub fn write_ir_non_recursive(
881877
};
882878

883879
// Fused cross + filter (show as nested loop join)
884-
if let Some(JoinTypeOptionsIR::CrossAndFilter {
885-
predicate,
886-
maintain_order: _,
887-
}) = &options.options
888-
{
880+
if let Some(JoinTypeOptionsIR::CrossAndFilter { predicate }) = &options.options {
889881
let predicate = predicate.display(expr_arena);
890882
write!(f, "{:indent$}NESTED_LOOP JOIN ON {predicate}", "")?;
891883
} else {

‎crates/polars-plan/src/plans/ir/inputs.rs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,7 @@ impl IR {
9090
options,
9191
..
9292
} => match &options.options {
93-
Some(JoinTypeOptionsIR::CrossAndFilter {
94-
predicate,
95-
maintain_order: _,
96-
}) => Exprs::Boxed(Box::new(
93+
Some(JoinTypeOptionsIR::CrossAndFilter { predicate }) => Exprs::Boxed(Box::new(
9794
left_on
9895
.iter()
9996
.chain(right_on.iter())
@@ -166,10 +163,7 @@ impl IR {
166163
options,
167164
..
168165
} => match Arc::make_mut(options).options.as_mut() {
169-
Some(JoinTypeOptionsIR::CrossAndFilter {
170-
predicate,
171-
maintain_order: _,
172-
}) => ExprsMut::Boxed(Box::new(
166+
Some(JoinTypeOptionsIR::CrossAndFilter { predicate }) => ExprsMut::Boxed(Box::new(
173167
left_on
174168
.iter_mut()
175169
.chain(right_on.iter_mut())

‎crates/polars-plan/src/plans/optimizer/predicate_pushdown/join.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,8 @@ fn try_rewrite_join_type(
409409

410410
match &options.options {
411411
Some(JoinTypeOptionsIR::CrossAndFilter { .. }) => {
412-
let Some(JoinTypeOptionsIR::CrossAndFilter {
413-
predicate,
414-
maintain_order: _,
415-
}) = Arc::make_mut(options).options.take()
412+
let Some(JoinTypeOptionsIR::CrossAndFilter { predicate }) =
413+
Arc::make_mut(options).options.take()
416414
else {
417415
unreachable!()
418416
};
@@ -537,12 +535,10 @@ fn try_rewrite_join_type(
537535
return Ok(());
538536
};
539537

540-
let maintain_order = options.args.maintain_order;
541538
let existing = Arc::make_mut(options)
542539
.options
543540
.replace(JoinTypeOptionsIR::CrossAndFilter {
544541
predicate: ExprIR::from_node(nested_loop_predicates, expr_arena),
545-
maintain_order,
546542
});
547543
assert!(existing.is_none()); // Important
548544

0 commit comments

Comments
 (0)