findepi commented on code in PR #14567:
URL: https://github.com/apache/datafusion/pull/14567#discussion_r1952641084
##########
datafusion/physical-optimizer/src/pruning.rs:
##########
@@ -1710,6 +1711,76 @@ fn build_like_match(
Some(combined)
}
+// For predicate `col NOT LIKE 'const_prefix%'`, we rewrite it as `(col_min
NOT LIKE 'const_prefix%' OR col_max NOT LIKE 'const_prefix%')`. If both col_min
and col_max have the prefix const_prefix, we skip the entire row group (as we
can be certain that all data in this row group has the prefix const_prefix).
+fn build_not_like_match(
+ expr_builder: &mut PruningExpressionBuilder<'_>,
+) -> Result<Arc<dyn PhysicalExpr>> {
+ // col NOT LIKE 'const_prefix%' -> !(col_min LIKE 'const_prefix%' &&
col_max LIKE 'const_prefix%') -> (col_min NOT LIKE 'const_prefix%' || col_max
NOT LIKE 'const_prefix%')
Review Comment:
this is not true either
the truth is (or should be):
```
col NOT LIKE 'const_prefix%' ->
col_max < 'const_prefix' OR col_min >= 'const_prefiy'
(notice the last character of the rightmost constant is different)
```
##########
datafusion/physical-optimizer/src/pruning.rs:
##########
@@ -1710,6 +1711,76 @@ fn build_like_match(
Some(combined)
}
+// For predicate `col NOT LIKE 'const_prefix%'`, we rewrite it as `(col_min
NOT LIKE 'const_prefix%' OR col_max NOT LIKE 'const_prefix%')`. If both col_min
and col_max have the prefix const_prefix, we skip the entire row group (as we
can be certain that all data in this row group has the prefix const_prefix).
Review Comment:
This is entirely not true. Please update to reflect actual logic
##########
datafusion/physical-optimizer/src/pruning.rs:
##########
@@ -1710,6 +1711,76 @@ fn build_like_match(
Some(combined)
}
+// For predicate `col NOT LIKE 'const_prefix%'`, we rewrite it as `(col_min
NOT LIKE 'const_prefix%' OR col_max NOT LIKE 'const_prefix%')`. If both col_min
and col_max have the prefix const_prefix, we skip the entire row group (as we
can be certain that all data in this row group has the prefix const_prefix).
+fn build_not_like_match(
+ expr_builder: &mut PruningExpressionBuilder<'_>,
+) -> Result<Arc<dyn PhysicalExpr>> {
+ // col NOT LIKE 'const_prefix%' -> !(col_min LIKE 'const_prefix%' &&
col_max LIKE 'const_prefix%') -> (col_min NOT LIKE 'const_prefix%' || col_max
NOT LIKE 'const_prefix%')
+
+ let min_column_expr = expr_builder.min_column_expr()?;
+ let max_column_expr = expr_builder.max_column_expr()?;
+
+ let scalar_expr = expr_builder.scalar_expr();
+
+ let pattern = extract_string_literal(scalar_expr).ok_or_else(|| {
+ plan_datafusion_err!("cannot extract literal from NOT LIKE expression")
+ })?;
+
+ let (const_prefix, remaining) = split_constant_prefix(pattern);
+ if const_prefix.is_empty() || remaining != "%" {
+ // we can not handle `%` at the beginning or in the middle of the
pattern
+ // Example: For pattern "foo%bar", the row group might include values
like
+ // ["foobar", "food", "foodbar"], making it unsafe to prune.
+ // Even if the min/max values in the group (e.g., "foobar" and
"foodbar")
+ // match the pattern, intermediate values like "food" may not
+ // match the full pattern "foo%bar", making pruning unsafe.
+ // (truncate foo%bar to foo% have same problem)
+
+ // we can not handle pattern containing `_`
+ // Example: For pattern "foo_", row groups might contain ["fooa",
"fooaa", "foob"],
+ // which means not every row is guaranteed to match the pattern.
+ return Err(plan_datafusion_err!(
+ "NOT LIKE expressions only support constant_prefix+wildcard`%`"
+ ));
+ }
+
+ let min_col_not_like_epxr = Arc::new(phys_expr::LikeExpr::new(
+ true,
+ false,
+ Arc::clone(&min_column_expr),
+ Arc::clone(scalar_expr),
Review Comment:
This should be
col_min >= 'const_prefiy'
(and renamed)
##########
datafusion/physical-optimizer/src/pruning.rs:
##########
@@ -1710,6 +1711,76 @@ fn build_like_match(
Some(combined)
}
+// For predicate `col NOT LIKE 'const_prefix%'`, we rewrite it as `(col_min
NOT LIKE 'const_prefix%' OR col_max NOT LIKE 'const_prefix%')`. If both col_min
and col_max have the prefix const_prefix, we skip the entire row group (as we
can be certain that all data in this row group has the prefix const_prefix).
+fn build_not_like_match(
+ expr_builder: &mut PruningExpressionBuilder<'_>,
+) -> Result<Arc<dyn PhysicalExpr>> {
+ // col NOT LIKE 'const_prefix%' -> !(col_min LIKE 'const_prefix%' &&
col_max LIKE 'const_prefix%') -> (col_min NOT LIKE 'const_prefix%' || col_max
NOT LIKE 'const_prefix%')
+
+ let min_column_expr = expr_builder.min_column_expr()?;
+ let max_column_expr = expr_builder.max_column_expr()?;
+
+ let scalar_expr = expr_builder.scalar_expr();
+
+ let pattern = extract_string_literal(scalar_expr).ok_or_else(|| {
+ plan_datafusion_err!("cannot extract literal from NOT LIKE expression")
+ })?;
+
+ let (const_prefix, remaining) = split_constant_prefix(pattern);
+ if const_prefix.is_empty() || remaining != "%" {
+ // we can not handle `%` at the beginning or in the middle of the
pattern
+ // Example: For pattern "foo%bar", the row group might include values
like
+ // ["foobar", "food", "foodbar"], making it unsafe to prune.
+ // Even if the min/max values in the group (e.g., "foobar" and
"foodbar")
+ // match the pattern, intermediate values like "food" may not
+ // match the full pattern "foo%bar", making pruning unsafe.
+ // (truncate foo%bar to foo% have same problem)
+
+ // we can not handle pattern containing `_`
+ // Example: For pattern "foo_", row groups might contain ["fooa",
"fooaa", "foob"],
+ // which means not every row is guaranteed to match the pattern.
Review Comment:
This is a good explanation for the code we didn't write (or we did, but it
was deleted).
This can be written more teresly as
```suggestion
// We handle `NOT LIKE '<constant-prefix>%'` case only, because only
this case
// can be converted to range predicates that can be then compared
with row group min/max values.
```
##########
datafusion/physical-optimizer/src/pruning.rs:
##########
@@ -1710,6 +1711,76 @@ fn build_like_match(
Some(combined)
}
+// For predicate `col NOT LIKE 'const_prefix%'`, we rewrite it as `(col_min
NOT LIKE 'const_prefix%' OR col_max NOT LIKE 'const_prefix%')`. If both col_min
and col_max have the prefix const_prefix, we skip the entire row group (as we
can be certain that all data in this row group has the prefix const_prefix).
+fn build_not_like_match(
+ expr_builder: &mut PruningExpressionBuilder<'_>,
+) -> Result<Arc<dyn PhysicalExpr>> {
+ // col NOT LIKE 'const_prefix%' -> !(col_min LIKE 'const_prefix%' &&
col_max LIKE 'const_prefix%') -> (col_min NOT LIKE 'const_prefix%' || col_max
NOT LIKE 'const_prefix%')
+
+ let min_column_expr = expr_builder.min_column_expr()?;
+ let max_column_expr = expr_builder.max_column_expr()?;
+
+ let scalar_expr = expr_builder.scalar_expr();
+
+ let pattern = extract_string_literal(scalar_expr).ok_or_else(|| {
+ plan_datafusion_err!("cannot extract literal from NOT LIKE expression")
+ })?;
+
+ let (const_prefix, remaining) = split_constant_prefix(pattern);
+ if const_prefix.is_empty() || remaining != "%" {
+ // we can not handle `%` at the beginning or in the middle of the
pattern
+ // Example: For pattern "foo%bar", the row group might include values
like
+ // ["foobar", "food", "foodbar"], making it unsafe to prune.
+ // Even if the min/max values in the group (e.g., "foobar" and
"foodbar")
+ // match the pattern, intermediate values like "food" may not
+ // match the full pattern "foo%bar", making pruning unsafe.
+ // (truncate foo%bar to foo% have same problem)
+
+ // we can not handle pattern containing `_`
+ // Example: For pattern "foo_", row groups might contain ["fooa",
"fooaa", "foob"],
+ // which means not every row is guaranteed to match the pattern.
+ return Err(plan_datafusion_err!(
+ "NOT LIKE expressions only support constant_prefix+wildcard`%`"
+ ));
+ }
+
+ let min_col_not_like_epxr = Arc::new(phys_expr::LikeExpr::new(
+ true,
+ false,
+ Arc::clone(&min_column_expr),
+ Arc::clone(scalar_expr),
+ ));
+
+ let max_col_not_like_expr = Arc::new(phys_expr::LikeExpr::new(
+ true,
+ false,
+ Arc::clone(&max_column_expr),
+ Arc::clone(scalar_expr),
+ ));
Review Comment:
This should be
col_max < 'const_prefix'
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
