waitingkuo commented on code in PR #7844:
URL: https://github.com/apache/arrow-datafusion/pull/7844#discussion_r1367633556
##########
datafusion/expr/src/built_in_function.rs:
##########
@@ -748,6 +751,7 @@ impl BuiltinScalarFunction {
BuiltinScalarFunction::ToTimestamp => Ok(Timestamp(Nanosecond,
None)),
Review Comment:
@alamb @comphead
any idea why the function could return `Timestamp(Second, None)` even
though we set the return type here as `Timestamp(Nanosecond, None)`?
##########
datafusion/physical-expr/src/functions.rs:
##########
@@ -74,15 +74,20 @@ pub fn create_physical_expr(
// so we don't have to pay a per-array/batch cost.
BuiltinScalarFunction::ToTimestamp => {
Arc::new(match input_phy_exprs[0].data_type(input_schema) {
- Ok(DataType::Int64) | Ok(DataType::Timestamp(_, None)) => {
- |col_values: &[ColumnarValue]| {
- cast_column(
- &col_values[0],
- &DataType::Timestamp(TimeUnit::Nanosecond, None),
- None,
- )
- }
- }
+ Ok(DataType::Int64) => |col_values: &[ColumnarValue]| {
+ cast_column(
+ &col_values[0],
+ &DataType::Timestamp(TimeUnit::Second, None),
+ None,
+ )
+ },
Review Comment:
it still returns Timestamp(second, None) while the input is Int64
```bash
❯ select arrow_typeof(to_timestamp(0));
+--------------------------------------+
| arrow_typeof(to_timestamp(Int64(0))) |
+--------------------------------------+
| Timestamp(Second, None) |
+--------------------------------------+
1 row in set. Query took 0.003 seconds.
```
##########
datafusion/physical-expr/src/functions.rs:
##########
@@ -74,15 +74,20 @@ pub fn create_physical_expr(
// so we don't have to pay a per-array/batch cost.
BuiltinScalarFunction::ToTimestamp => {
Arc::new(match input_phy_exprs[0].data_type(input_schema) {
- Ok(DataType::Int64) | Ok(DataType::Timestamp(_, None)) => {
- |col_values: &[ColumnarValue]| {
- cast_column(
- &col_values[0],
- &DataType::Timestamp(TimeUnit::Nanosecond, None),
- None,
- )
- }
- }
+ Ok(DataType::Int64) => |col_values: &[ColumnarValue]| {
+ cast_column(
+ &col_values[0],
+ &DataType::Timestamp(TimeUnit::Second, None),
+ None,
+ )
+ },
Review Comment:
it still returns Timestamp(second, None) while the input is Int64
```bash
❯ select arrow_typeof(to_timestamp(0));
+--------------------------------------+
| arrow_typeof(to_timestamp(Int64(0))) |
+--------------------------------------+
| Timestamp(Second, None) |
+--------------------------------------+
1 row in set. Query took 0.003 seconds.
```
--
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]
