houqp commented on a change in pull request #1030:
URL: https://github.com/apache/arrow-datafusion/pull/1030#discussion_r712691875
##########
File path: datafusion/src/logical_plan/expr.rs
##########
@@ -1408,6 +1452,11 @@ pub fn lit<T: Literal>(n: T) -> Expr {
n.lit()
}
+/// Create a literal timestamp expression
+pub fn lit_timestamp_nanosecond<T: Literal>(n: T) -> Expr {
Review comment:
do we consider it a user error if a non-numerical literal type is passed
in here? If so, it might be better to introduce a new ns timestamp trait and
only implement it for the numeric types we support so we can let rust compiler
catch these issues at build time.
##########
File path: datafusion/src/logical_plan/expr.rs
##########
@@ -1387,6 +1426,11 @@ macro_rules! make_literal {
fn lit(&self) -> Expr {
Expr::Literal(ScalarValue::$SCALAR(Some(self.clone())))
}
+
+ fn lit_timestamp_nanosecond(&self) -> Expr {
+ let scalar = ScalarValue::$SCALAR(Some(self.clone()));
+ scalar.lit_timestamp_nanosecond()
Review comment:
I think this seems a little bit inefficient to wrap a numeric value with
`ScalarValue` enum, then subsequently match on that enum to unpack the value
and return a different enum. Perhaps we could find a way to execute
`ScalarValue::TimestampNanosecond(Some(self.clone().into())` here directly?
--
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]
