Improve filter predicates with Utf8View literals

[Weijun-H]

Closes #10998

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

[alamb]

Thanks @Weijun-H -- I think you might be able to do more than just scalars. I left a suggestion

[alamb]

I think the previous logic that calls get_input_types should handle all types of Exprs, including scalars.

Thus I am not sure we need special case logic here for just scalars (which won't work for other argument types)

Did you look into extending get_input_types to handle Utf8Views?

[Weijun-H]

Yes, I did look at that. But the problem is that get_input_types only handles DataType instead of Expr

/// Returns the coerced input types for a binary expression evaluating the `op` with the left and right hand types
pub fn get_input_types(
    lhs: &DataType,
    op: &Operator,
    rhs: &DataType,
) -> Result<(DataType, DataType)> {
    signature(lhs, op, rhs).map(|sig| (sig.lhs, sig.rhs))
}

[XiangpengHao]

I think @Weijun-H is right that we need to consider not only coercion by data type but also their column types.
Normally:
(Utf8View, Utf8) => (Utf8View, Utf8View)

But:
(Scalar(Utf8View), Column(Utf8)) => (Scalar(Utf8), Column(Utf8))

I'll take a closer look and think about what is the best way to approach this..

[alamb]

🤔 I wonder if this might be a good candidate for extending the UnwrapCastInComparison logic, which already handles similar cases with scalars

datafusion/datafusion/optimizer/src/unwrap_cast_in_comparison.rs

Lines 38 to 74 in ed7c884

	/// [`UnwrapCastInComparison`] attempts to remove casts from
	/// comparisons to literals ([`ScalarValue`]s) by applying the casts
	/// to the literals if possible. It is inspired by the optimizer rule
	/// `UnwrapCastInBinaryComparison` of Spark.
	///
	/// Removing casts often improves performance because:
	/// 1. The cast is done once (to the literal) rather than to every value
	/// 2. Can enable other optimizations such as predicate pushdown that
	/// don't support casting
	///
	/// The rule is applied to expressions of the following forms:
	///
	/// 1. `cast(left_expr as data_type) comparison_op literal_expr`
	/// 2. `literal_expr comparison_op cast(left_expr as data_type)`
	/// 3. `cast(literal_expr) IN (expr1, expr2, ...)`
	/// 4. `literal_expr IN (cast(expr1) , cast(expr2), ...)`
	///
	/// If the expression matches one of the forms above, the rule will
	/// ensure the value of `literal` is in range(min, max) of the
	/// expr's data_type, and if the scalar is within range, the literal
	/// will be casted to the data type of expr on the other side, and the
	/// cast will be removed from the other side.
	///
	/// # Example
	///
	/// If the DataType of c1 is INT32. Given the filter
	///
	/// ```text
	/// Filter: cast(c1 as INT64) > INT64(10)`
	/// ```
	///
	/// This rule will remove the cast and rewrite the expression to:
	///
	/// ```text
	/// Filter: c1 > INT32(10)
	/// ```
	///

[Weijun-H]

It could be a workaround, I will look into it.

[alamb]

This looks perfect @Weijun-H -- thank you 🙏

[alamb]

Nice cleanup

[alamb]

Thanks again @Weijun-H