alamb commented on code in PR #9790:
URL: https://github.com/apache/arrow-datafusion/pull/9790#discussion_r1540403796
##########
datafusion/sql/src/query.rs:
##########
@@ -283,3 +276,44 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}
}
}
+
+/// Retrieves the constant result of an expression, evaluating it if possible.
+///
+/// This function takes an expression and an argument name as input and returns
+/// a `Result<i64>` indicating either the constant result of the expression or
an
+/// error if the expression cannot be evaluated.
+///
+/// # Arguments
+///
+/// * `expr` - An `Expr` representing the expression to evaluate.
+/// * `arg_name` - The name of the argument for error messages.
+///
+/// # Returns
+///
+/// * `Result<i64>` - An `Ok` variant containing the constant result if
evaluation is successful,
+/// or an `Err` variant containing an error message if evaluation fails.
+fn get_constant_result(expr: &Expr, arg_name: &str) -> Result<i64> {
+ match expr {
Review Comment:
> Could we change the Limit logical plan to support arbitrary expressions?
I agree this would be the "correct" way to support arbitrarily simplifiable
expressions in the limit clause
However, I suspect it might be a major change
So I guess it comes down to "how important is the `+` / `-` case -- if it is
really important then we can proceed with this PR, but if the real use case is
arbitrary expressions, it probably makes sense to do the more substantial
change as suggested by @jonahgao
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
