Skip to content

Commit 8c7e29c

Browse files
b41shMazterQyou
authored andcommitted
Refactor Expr::Like, Expr::ILike, Expr::SimilarTo to use a struct (apache#3836)
* Refactor `Expr::Like`, `Expr::ILike`, `Expr::SimilarTo` to use a struct * optimize type * fix clippy
1 parent 63baa7c commit 8c7e29c

File tree

16 files changed

+189
-193
lines changed

16 files changed

+189
-193
lines changed

datafusion/core/src/logical_plan/expr.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub use datafusion_common::{Column, ExprSchema};
2828
pub use datafusion_expr::expr_fn::*;
2929
use datafusion_expr::BuiltinScalarFunction;
3030
pub use datafusion_expr::Expr;
31+
pub use datafusion_expr::Like;
3132
use datafusion_expr::StateTypeFunction;
3233
pub use datafusion_expr::{lit, lit_timestamp_nano, Literal};
3334
use datafusion_expr::{

datafusion/core/src/logical_plan/expr_rewriter.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
//! Expression rewriter
1919
20-
use super::{Expr, ExprSchemable};
20+
use super::{Expr, ExprSchemable, Like};
2121
use crate::logical_plan::plan::{Aggregate, Projection};
2222
use crate::logical_plan::DFSchema;
2323
use crate::logical_plan::LogicalPlan;
@@ -127,39 +127,39 @@ impl ExprRewritable for Expr {
127127
op,
128128
right: rewrite_boxed(right, rewriter)?,
129129
},
130-
Expr::Like {
130+
Expr::Like(Like {
131131
negated,
132132
expr,
133133
pattern,
134134
escape_char,
135-
} => Expr::Like {
135+
}) => Expr::Like(Like::new(
136136
negated,
137-
expr: rewrite_boxed(expr, rewriter)?,
138-
pattern: rewrite_boxed(pattern, rewriter)?,
137+
rewrite_boxed(expr, rewriter)?,
138+
rewrite_boxed(pattern, rewriter)?,
139139
escape_char,
140-
},
141-
Expr::ILike {
140+
)),
141+
Expr::ILike(Like {
142142
negated,
143143
expr,
144144
pattern,
145145
escape_char,
146-
} => Expr::ILike {
146+
}) => Expr::ILike(Like::new(
147147
negated,
148-
expr: rewrite_boxed(expr, rewriter)?,
149-
pattern: rewrite_boxed(pattern, rewriter)?,
148+
rewrite_boxed(expr, rewriter)?,
149+
rewrite_boxed(pattern, rewriter)?,
150150
escape_char,
151-
},
152-
Expr::SimilarTo {
151+
)),
152+
Expr::SimilarTo(Like {
153153
negated,
154154
expr,
155155
pattern,
156156
escape_char,
157-
} => Expr::SimilarTo {
157+
}) => Expr::SimilarTo(Like::new(
158158
negated,
159-
expr: rewrite_boxed(expr, rewriter)?,
160-
pattern: rewrite_boxed(pattern, rewriter)?,
159+
rewrite_boxed(expr, rewriter)?,
160+
rewrite_boxed(pattern, rewriter)?,
161161
escape_char,
162-
},
162+
)),
163163
Expr::Not(expr) => Expr::Not(rewrite_boxed(expr, rewriter)?),
164164
Expr::IsNotNull(expr) => Expr::IsNotNull(rewrite_boxed(expr, rewriter)?),
165165
Expr::IsNull(expr) => Expr::IsNull(rewrite_boxed(expr, rewriter)?),

datafusion/core/src/logical_plan/expr_schema.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18-
use super::Expr;
18+
use super::{Expr, Like};
1919
use crate::physical_plan::{
2020
aggregates, expressions::binary_operator_data_type, functions, window_functions,
2121
};
@@ -198,9 +198,9 @@ impl ExprSchemable for Expr {
198198
ref right,
199199
..
200200
} => Ok(left.nullable(input_schema)? || right.nullable(input_schema)?),
201-
Expr::Like { expr, .. } => expr.nullable(input_schema),
202-
Expr::ILike { expr, .. } => expr.nullable(input_schema),
203-
Expr::SimilarTo { expr, .. } => expr.nullable(input_schema),
201+
Expr::Like(Like { expr, .. }) => expr.nullable(input_schema),
202+
Expr::ILike(Like { expr, .. }) => expr.nullable(input_schema),
203+
Expr::SimilarTo(Like { expr, .. }) => expr.nullable(input_schema),
204204
Expr::Wildcard => Err(DataFusionError::Internal(
205205
"Wildcard expressions are not valid in a logical query plan".to_owned(),
206206
)),

datafusion/core/src/logical_plan/expr_visitor.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
//! Expression visitor
1919
20-
use super::Expr;
20+
use super::{Expr, Like};
2121
use datafusion_common::Result;
2222

2323
/// Controls how the visitor recursion should proceed.
@@ -120,15 +120,15 @@ impl ExprVisitable for Expr {
120120
let visitor = left.accept(visitor)?;
121121
right.accept(visitor)
122122
}
123-
Expr::Like { expr, pattern, .. } => {
123+
Expr::Like(Like { expr, pattern, .. }) => {
124124
let visitor = expr.accept(visitor)?;
125125
pattern.accept(visitor)
126126
}
127-
Expr::ILike { expr, pattern, .. } => {
127+
Expr::ILike(Like { expr, pattern, .. }) => {
128128
let visitor = expr.accept(visitor)?;
129129
pattern.accept(visitor)
130130
}
131-
Expr::SimilarTo { expr, pattern, .. } => {
131+
Expr::SimilarTo(Like { expr, pattern, .. }) => {
132132
let visitor = expr.accept(visitor)?;
133133
pattern.accept(visitor)
134134
}

datafusion/core/src/logical_plan/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ pub use expr::{
5252
replace, reverse, right, round, rpad, rtrim, sha224, sha256, sha384, sha512, signum,
5353
sin, split_part, sqrt, starts_with, strpos, substr, sum, tan, to_hex,
5454
to_timestamp_micros, to_timestamp_millis, to_timestamp_seconds, translate, trim,
55-
trunc, unalias, upper, when, Column, Expr, ExprSchema, Literal,
55+
trunc, unalias, upper, when, Column, Expr, ExprSchema, Like, Literal,
5656
};
5757
pub use expr_rewriter::{
5858
normalize_col, normalize_cols, replace_col, rewrite_sort_cols_by_aggs,

datafusion/core/src/optimizer/common_subexpr_eliminate.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::logical_plan::{
2323
col,
2424
plan::{Aggregate, Sort},
2525
DFField, DFSchema, Expr, ExprRewritable, ExprRewriter, ExprSchemable, ExprVisitable,
26-
ExpressionVisitor, LogicalPlan, Recursion, RewriteRecursion,
26+
ExpressionVisitor, Like, LogicalPlan, Recursion, RewriteRecursion,
2727
};
2828
use crate::optimizer::optimizer::OptimizerConfig;
2929
use crate::optimizer::optimizer::OptimizerRule;
@@ -447,15 +447,15 @@ impl ExprIdentifierVisitor<'_> {
447447
desc.push_str("Between-");
448448
desc.push_str(&negated.to_string());
449449
}
450-
Expr::Like { negated, .. } => {
450+
Expr::Like(Like { negated, .. }) => {
451451
desc.push_str("Like-");
452452
desc.push_str(&negated.to_string());
453453
}
454-
Expr::ILike { negated, .. } => {
454+
Expr::ILike(Like { negated, .. }) => {
455455
desc.push_str("ILike-");
456456
desc.push_str(&negated.to_string());
457457
}
458-
Expr::SimilarTo { negated, .. } => {
458+
Expr::SimilarTo(Like { negated, .. }) => {
459459
desc.push_str("SimilarTo-");
460460
desc.push_str(&negated.to_string());
461461
}

datafusion/core/src/optimizer/utils.rs

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::optimizer::optimizer::OptimizerConfig;
2727
use crate::logical_plan::builder::build_table_udf_schema;
2828
use crate::logical_plan::{
2929
build_join_schema, Column, CreateMemoryTable, DFSchemaRef, Distinct, Expr,
30-
ExprVisitable, Limit, LogicalPlan, LogicalPlanBuilder, Operator, Partitioning,
30+
ExprVisitable, Like, Limit, LogicalPlan, LogicalPlanBuilder, Operator, Partitioning,
3131
Recursion, Repartition, Union, Values,
3232
};
3333
use crate::prelude::lit;
@@ -316,9 +316,9 @@ pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<Expr>> {
316316
Expr::AnyExpr { left, right, .. } => {
317317
Ok(vec![left.as_ref().to_owned(), right.as_ref().to_owned()])
318318
}
319-
Expr::Like { expr, pattern, .. }
320-
| Expr::ILike { expr, pattern, .. }
321-
| Expr::SimilarTo { expr, pattern, .. } => {
319+
Expr::Like(Like { expr, pattern, .. })
320+
| Expr::ILike(Like { expr, pattern, .. })
321+
| Expr::SimilarTo(Like { expr, pattern, .. }) => {
322322
Ok(vec![expr.as_ref().to_owned(), pattern.as_ref().to_owned()])
323323
}
324324
Expr::IsNull(expr)
@@ -415,36 +415,36 @@ pub fn rewrite_expression(expr: &Expr, expressions: &[Expr]) -> Result<Expr> {
415415
op: *op,
416416
right: Box::new(expressions[1].clone()),
417417
}),
418-
Expr::Like {
418+
Expr::Like(Like {
419419
negated,
420420
escape_char,
421421
..
422-
} => Ok(Expr::Like {
423-
negated: *negated,
424-
expr: Box::new(expressions[0].clone()),
425-
pattern: Box::new(expressions[1].clone()),
426-
escape_char: *escape_char,
427-
}),
428-
Expr::ILike {
422+
}) => Ok(Expr::Like(Like::new(
423+
*negated,
424+
Box::new(expressions[0].clone()),
425+
Box::new(expressions[1].clone()),
426+
*escape_char,
427+
))),
428+
Expr::ILike(Like {
429429
negated,
430430
escape_char,
431431
..
432-
} => Ok(Expr::ILike {
433-
negated: *negated,
434-
expr: Box::new(expressions[0].clone()),
435-
pattern: Box::new(expressions[1].clone()),
436-
escape_char: *escape_char,
437-
}),
438-
Expr::SimilarTo {
432+
}) => Ok(Expr::ILike(Like::new(
433+
*negated,
434+
Box::new(expressions[0].clone()),
435+
Box::new(expressions[1].clone()),
436+
*escape_char,
437+
))),
438+
Expr::SimilarTo(Like {
439439
negated,
440440
escape_char,
441441
..
442-
} => Ok(Expr::SimilarTo {
443-
negated: *negated,
444-
expr: Box::new(expressions[0].clone()),
445-
pattern: Box::new(expressions[1].clone()),
446-
escape_char: *escape_char,
447-
}),
442+
}) => Ok(Expr::SimilarTo(Like::new(
443+
*negated,
444+
Box::new(expressions[0].clone()),
445+
Box::new(expressions[1].clone()),
446+
*escape_char,
447+
))),
448448
Expr::IsNull(_) => Ok(Expr::IsNull(Box::new(expressions[0].clone()))),
449449
Expr::IsNotNull(_) => Ok(Expr::IsNotNull(Box::new(expressions[0].clone()))),
450450
Expr::ScalarFunction { fun, .. } => Ok(Expr::ScalarFunction {

datafusion/core/src/physical_plan/planner.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use crate::logical_plan::plan::{
3030
TableUDFs, Window,
3131
};
3232
use crate::logical_plan::{
33-
unalias, unnormalize_cols, CrossJoin, DFSchema, Distinct, Expr, LogicalPlan,
33+
unalias, unnormalize_cols, CrossJoin, DFSchema, Distinct, Expr, Like, LogicalPlan,
3434
Operator, Partitioning as LogicalPartitioning, PlanType, Repartition,
3535
ToStringifiedPlan, Union, UserDefinedLogicalNode,
3636
};
@@ -219,12 +219,12 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
219219
Ok(format!("{} BETWEEN {} AND {}", expr, low, high))
220220
}
221221
}
222-
Expr::Like {
222+
Expr::Like(Like {
223223
negated,
224224
expr,
225225
pattern,
226226
escape_char,
227-
} => {
227+
}) => {
228228
let expr = create_physical_name(expr, false)?;
229229
let pattern = create_physical_name(pattern, false)?;
230230
let escape = if let Some(char) = escape_char {
@@ -238,12 +238,12 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
238238
Ok(format!("{} LIKE {}{}", expr, pattern, escape))
239239
}
240240
}
241-
Expr::ILike {
241+
Expr::ILike(Like {
242242
negated,
243243
expr,
244244
pattern,
245245
escape_char,
246-
} => {
246+
}) => {
247247
let expr = create_physical_name(expr, false)?;
248248
let pattern = create_physical_name(pattern, false)?;
249249
let escape = if let Some(char) = escape_char {
@@ -257,12 +257,12 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
257257
Ok(format!("{} ILIKE {}{}", expr, pattern, escape))
258258
}
259259
}
260-
Expr::SimilarTo {
260+
Expr::SimilarTo(Like {
261261
negated,
262262
expr,
263263
pattern,
264264
escape_char,
265-
} => {
265+
}) => {
266266
let expr = create_physical_name(expr, false)?;
267267
let pattern = create_physical_name(pattern, false)?;
268268
let escape = if let Some(char) = escape_char {
@@ -1081,12 +1081,12 @@ pub fn create_physical_expr(
10811081
)?;
10821082
binary(lhs, *op, rhs, input_schema)
10831083
}
1084-
Expr::Like {
1084+
Expr::Like(Like {
10851085
negated,
10861086
expr,
10871087
pattern,
10881088
escape_char,
1089-
} => {
1089+
}) => {
10901090
if escape_char.is_some() {
10911091
return Err(DataFusionError::Execution(
10921092
"LIKE does not support escape_char".to_string(),
@@ -1101,12 +1101,12 @@ pub fn create_physical_expr(
11011101
binary_expr(expr.as_ref().clone(), op, pattern.as_ref().clone());
11021102
create_physical_expr(&bin_expr, input_dfschema, input_schema, execution_props)
11031103
}
1104-
Expr::ILike {
1104+
Expr::ILike(Like {
11051105
negated,
11061106
expr,
11071107
pattern,
11081108
escape_char,
1109-
} => {
1109+
}) => {
11101110
if escape_char.is_some() {
11111111
return Err(DataFusionError::Execution(
11121112
"ILIKE does not support escape_char".to_string(),

datafusion/core/src/row/writer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ impl RowWriter {
264264
fn set_bool(&mut self, idx: usize, value: bool) {
265265
self.assert_index_valid(idx);
266266
let offset = self.field_offsets[idx];
267-
self.data[offset] = if value { 1 } else { 0 };
267+
self.data[offset] = u8::from(value);
268268
}
269269

270270
fn set_u8(&mut self, idx: usize, value: u8) {

datafusion/core/src/sql/planner.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use crate::logical_plan::Expr::Alias;
3131
use crate::logical_plan::{
3232
and, builder::expand_qualified_wildcard, builder::expand_wildcard, col, lit,
3333
normalize_col, rewrite_udtfs_to_columns, Column, CreateMemoryTable, DFSchema,
34-
DFSchemaRef, DropTable, Expr, ExprSchemable, LogicalPlan, LogicalPlanBuilder,
34+
DFSchemaRef, DropTable, Expr, ExprSchemable, Like, LogicalPlan, LogicalPlanBuilder,
3535
Operator, PlanType, ToDFSchema, ToStringifiedPlan,
3636
};
3737
use crate::optimizer::utils::exprlist_to_columns;
@@ -2009,13 +2009,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
20092009
"Invalid pattern in LIKE expression".to_string(),
20102010
));
20112011
}
2012-
Ok(Expr::Like {
2012+
Ok(Expr::Like(Like::new(
20132013
negated,
2014-
expr: Box::new(self.sql_expr_to_logical_expr(*expr, schema)?),
2015-
pattern: Box::new(pattern),
2014+
Box::new(self.sql_expr_to_logical_expr(*expr, schema)?),
2015+
Box::new(pattern),
20162016
escape_char
2017-
2018-
})
2017+
)))
20192018
}
20202019

20212020
SQLExpr::ILike { negated, expr, pattern, escape_char } => {
@@ -2026,12 +2025,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
20262025
"Invalid pattern in ILIKE expression".to_string(),
20272026
));
20282027
}
2029-
Ok(Expr::ILike {
2028+
Ok(Expr::ILike(Like::new(
20302029
negated,
2031-
expr: Box::new(self.sql_expr_to_logical_expr(*expr, schema)?),
2032-
pattern: Box::new(pattern),
2030+
Box::new(self.sql_expr_to_logical_expr(*expr, schema)?),
2031+
Box::new(pattern),
20332032
escape_char
2034-
})
2033+
)))
20352034
}
20362035

20372036
SQLExpr::SimilarTo { negated, expr, pattern, escape_char } => {
@@ -2042,12 +2041,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
20422041
"Invalid pattern in SIMILAR TO expression".to_string(),
20432042
));
20442043
}
2045-
Ok(Expr::SimilarTo {
2044+
Ok(Expr::SimilarTo(Like::new(
20462045
negated,
2047-
expr: Box::new(self.sql_expr_to_logical_expr(*expr, schema)?),
2048-
pattern: Box::new(pattern),
2046+
Box::new(self.sql_expr_to_logical_expr(*expr, schema)?),
2047+
Box::new(pattern),
20492048
escape_char
2050-
})
2049+
)))
20512050
}
20522051

20532052
SQLExpr::BinaryOp {

0 commit comments

Comments
 (0)