Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing issues with for timestamp literals #8193

Merged
merged 9 commits into from
Nov 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions datafusion/common/src/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,18 @@ impl ScalarValue {
ScalarValue::Decimal256(Some(v), precision, scale) => Ok(
ScalarValue::Decimal256(Some(v.neg_wrapping()), *precision, *scale),
),
ScalarValue::TimestampSecond(Some(v), tz) => {
comphead marked this conversation as resolved.
Show resolved Hide resolved
Ok(ScalarValue::TimestampSecond(Some(-v), tz.clone()))
}
ScalarValue::TimestampNanosecond(Some(v), tz) => {
Ok(ScalarValue::TimestampNanosecond(Some(-v), tz.clone()))
}
ScalarValue::TimestampMicrosecond(Some(v), tz) => {
Ok(ScalarValue::TimestampMicrosecond(Some(-v), tz.clone()))
}
ScalarValue::TimestampMillisecond(Some(v), tz) => {
Ok(ScalarValue::TimestampMillisecond(Some(-v), tz.clone()))
}
value => _internal_err!(
"Can not run arithmetic negative on scalar value {value:?}"
),
Expand Down
2 changes: 1 addition & 1 deletion datafusion/core/tests/sql/timestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ async fn test_arrow_typeof() -> Result<()> {
"+-----------------------------------------------------------------------+",
"| arrow_typeof(date_trunc(Utf8(\"microsecond\"),to_timestamp(Int64(61)))) |",
"+-----------------------------------------------------------------------+",
"| Timestamp(Second, None) |",
"| Timestamp(Nanosecond, None) |",
"+-----------------------------------------------------------------------+",
];
assert_batches_eq!(expected, &actual);
Expand Down
7 changes: 2 additions & 5 deletions datafusion/expr/src/built_in_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,13 +779,10 @@ impl BuiltinScalarFunction {
BuiltinScalarFunction::SubstrIndex => {
utf8_to_str_type(&input_expr_types[0], "substr_index")
}
BuiltinScalarFunction::ToTimestamp => Ok(match &input_expr_types[0] {
Int64 => Timestamp(Second, None),
_ => Timestamp(Nanosecond, None),
}),
BuiltinScalarFunction::ToTimestamp
| BuiltinScalarFunction::ToTimestampNanos => Ok(Timestamp(Nanosecond, None)),
alamb marked this conversation as resolved.
Show resolved Hide resolved
BuiltinScalarFunction::ToTimestampMillis => Ok(Timestamp(Millisecond, None)),
BuiltinScalarFunction::ToTimestampMicros => Ok(Timestamp(Microsecond, None)),
BuiltinScalarFunction::ToTimestampNanos => Ok(Timestamp(Nanosecond, None)),
BuiltinScalarFunction::ToTimestampSeconds => Ok(Timestamp(Second, None)),
BuiltinScalarFunction::FromUnixtime => Ok(Timestamp(Second, None)),
BuiltinScalarFunction::Now => {
Expand Down
8 changes: 5 additions & 3 deletions datafusion/physical-expr/src/datetime_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,9 +966,11 @@ pub fn to_timestamp_invoke(args: &[ColumnarValue]) -> Result<ColumnarValue> {
}

match args[0].data_type() {
DataType::Int64 => {
cast_column(&args[0], &DataType::Timestamp(TimeUnit::Second, None), None)
}
DataType::Int64 => cast_column(
&cast_column(&args[0], &DataType::Timestamp(TimeUnit::Second, None), None)?,
&DataType::Timestamp(TimeUnit::Nanosecond, None),
None,
),
DataType::Timestamp(_, None) => cast_column(
&args[0],
&DataType::Timestamp(TimeUnit::Nanosecond, None),
Expand Down
7 changes: 5 additions & 2 deletions datafusion/physical-expr/src/expressions/negative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use arrow::{
use datafusion_common::{internal_err, DataFusionError, Result};
use datafusion_expr::interval_arithmetic::Interval;
use datafusion_expr::{
type_coercion::{is_interval, is_null, is_signed_numeric},
type_coercion::{is_interval, is_null, is_signed_numeric, is_timestamp},
ColumnarValue,
};

Expand Down Expand Up @@ -160,7 +160,10 @@ pub fn negative(
let data_type = arg.data_type(input_schema)?;
if is_null(&data_type) {
Ok(arg)
} else if !is_signed_numeric(&data_type) && !is_interval(&data_type) {
} else if !is_signed_numeric(&data_type)
&& !is_interval(&data_type)
&& !is_timestamp(&data_type)
{
internal_err!(
"Can't create negative physical expr for (- '{arg:?}'), the type of child expr is {data_type}, not signed numeric"
)
Expand Down
30 changes: 22 additions & 8 deletions datafusion/sql/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ mod value;

use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
use arrow_schema::DataType;
use arrow_schema::TimeUnit;
use datafusion_common::{
internal_err, not_impl_err, plan_err, Column, DFSchema, DataFusionError, Result,
ScalarValue,
Expand Down Expand Up @@ -224,14 +225,27 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {

SQLExpr::Cast {
expr, data_type, ..
} => Ok(Expr::Cast(Cast::new(
Box::new(self.sql_expr_to_logical_expr(
*expr,
schema,
planner_context,
)?),
self.convert_data_type(&data_type)?,
))),
} => {
let dt = self.convert_data_type(&data_type)?;
let expr =
self.sql_expr_to_logical_expr(*expr, schema, planner_context)?;

// numeric constants are treated as seconds (rather as nanoseconds)
// to align with postgres / duckdb semantics
let expr = match &dt {
DataType::Timestamp(TimeUnit::Nanosecond, tz)
if expr.get_type(schema)? == DataType::Int64 =>
{
Expr::Cast(Cast::new(
Box::new(expr),
DataType::Timestamp(TimeUnit::Second, tz.clone()),
))
}
_ => expr,
};

Ok(Expr::Cast(Cast::new(Box::new(expr), dt)))
}

SQLExpr::TryCast {
expr, data_type, ..
Expand Down
4 changes: 1 addition & 3 deletions datafusion/sql/tests/sql_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,11 +606,9 @@ fn select_compound_filter() {
#[test]
fn test_timestamp_filter() {
let sql = "SELECT state FROM person WHERE birth_date < CAST (158412331400600000 as timestamp)";

let expected = "Projection: person.state\
\n Filter: person.birth_date < CAST(Int64(158412331400600000) AS Timestamp(Nanosecond, None))\
\n Filter: person.birth_date < CAST(CAST(Int64(158412331400600000) AS Timestamp(Second, None)) AS Timestamp(Nanosecond, None))\
\n TableScan: person";

quick_test(sql, expected);
}

Expand Down
52 changes: 47 additions & 5 deletions datafusion/sqllogictest/test_files/timestamps.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1788,8 +1788,50 @@ SELECT TIMESTAMPTZ '2020-01-01 00:00:00Z' = TIMESTAMP '2020-01-01'
----
true

# verify to_timestamp edge cases to be in sync with postgresql
query PPPPP
SELECT to_timestamp(null), to_timestamp(-62125747200), to_timestamp(0), to_timestamp(1926632005177), to_timestamp(1926632005)
----
NULL 0001-04-25T00:00:00 1970-01-01T00:00:00 +63022-07-16T12:59:37 2031-01-19T23:33:25
# verify timestamp cast with integer input
query PPPPPP
SELECT to_timestamp(null), to_timestamp(0), to_timestamp(1926632005), to_timestamp(1), to_timestamp(-1), to_timestamp(0-1)
----
NULL 1970-01-01T00:00:00 2031-01-19T23:33:25 1970-01-01T00:00:01 1969-12-31T23:59:59 1969-12-31T23:59:59

# verify timestamp syntax stlyes are consistent
query BBBBBBBBBBBBB
SELECT to_timestamp(null) is null as c1,
null::timestamp is null as c2,
cast(null as timestamp) is null as c3,
to_timestamp(0) = 0::timestamp as c4,
to_timestamp(1926632005) = 1926632005::timestamp as c5,
to_timestamp(1) = 1::timestamp as c6,
to_timestamp(-1) = -1::timestamp as c7,
to_timestamp(0-1) = (0-1)::timestamp as c8,
to_timestamp(0) = cast(0 as timestamp) as c9,
to_timestamp(1926632005) = cast(1926632005 as timestamp) as c10,
to_timestamp(1) = cast(1 as timestamp) as c11,
to_timestamp(-1) = cast(-1 as timestamp) as c12,
to_timestamp(0-1) = cast(0-1 as timestamp) as c13
----
true true true true true true true true true true true true true

# verify timestamp output types
query TTT
SELECT arrow_typeof(to_timestamp(1)), arrow_typeof(to_timestamp(null)), arrow_typeof(to_timestamp('2023-01-10 12:34:56.000'))
----
Timestamp(Nanosecond, None) Timestamp(Nanosecond, None) Timestamp(Nanosecond, None)

# verify timestamp output types using timestamp literal syntax
query BBBBBB
SELECT arrow_typeof(to_timestamp(1)) = arrow_typeof(1::timestamp) as c1,
arrow_typeof(to_timestamp(null)) = arrow_typeof(null::timestamp) as c2,
arrow_typeof(to_timestamp('2023-01-10 12:34:56.000')) = arrow_typeof('2023-01-10 12:34:56.000'::timestamp) as c3,
arrow_typeof(to_timestamp(1)) = arrow_typeof(cast(1 as timestamp)) as c4,
arrow_typeof(to_timestamp(null)) = arrow_typeof(cast(null as timestamp)) as c5,
arrow_typeof(to_timestamp('2023-01-10 12:34:56.000')) = arrow_typeof(cast('2023-01-10 12:34:56.000' as timestamp)) as c6
----
true true true true true true

# known issues. currently overflows (expects default precision to be microsecond instead of nanoseconds. Work pending)
#verify extreme values
#query PPPPPPPP
#SELECT to_timestamp(-62125747200), to_timestamp(1926632005177), -62125747200::timestamp, 1926632005177::timestamp, cast(-62125747200 as timestamp), cast(1926632005177 as timestamp)
#----
#0001-04-25T00:00:00 +63022-07-16T12:59:37 0001-04-25T00:00:00 +63022-07-16T12:59:37 0001-04-25T00:00:00 +63022-07-16T12:59:37
16 changes: 8 additions & 8 deletions datafusion/sqllogictest/test_files/window.slt
Original file line number Diff line number Diff line change
Expand Up @@ -895,14 +895,14 @@ SELECT

statement ok
create table temp as values
(1664264591000000000),
(1664264592000000000),
(1664264592000000000),
(1664264593000000000),
(1664264594000000000),
(1664364594000000000),
(1664464594000000000),
(1664564594000000000);
(1664264591),
(1664264592),
(1664264592),
(1664264593),
(1664264594),
(1664364594),
(1664464594),
(1664564594);

statement ok
create table t as select cast(column1 as timestamp) as ts from temp;
Expand Down