From baf3eb9d62bea7b76c2d18f40a79a1fbe13a860e Mon Sep 17 00:00:00 2001 From: mwish Date: Mon, 14 Aug 2023 23:44:44 +0800 Subject: [PATCH] GH-37110: [C++] Expression: SmallestTypeFor lost tz for Scalar (#37135) ### Rationale for this change This patch ( https://github.com/apache/arrow/pull/15180 ) adds a `SmallestTypeFor` to handling expression type. However, it lost timezone when handling. ### What changes are included in this PR? Add `timezone` in `SmallestTypeFor` ### Are these changes tested? Currently not ### Are there any user-facing changes? Yeah it's a bugfix * Closes: #37110 Authored-by: mwish Signed-off-by: Antoine Pitrou --- cpp/src/arrow/compute/expression.cc | 12 ++++++------ cpp/src/arrow/compute/expression_test.cc | 13 +++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/compute/expression.cc b/cpp/src/arrow/compute/expression.cc index 247f4e540872f..b47e0a35525c5 100644 --- a/cpp/src/arrow/compute/expression.cc +++ b/cpp/src/arrow/compute/expression.cc @@ -480,26 +480,26 @@ TypeHolder SmallestTypeFor(const arrow::Datum& value) { return value.type(); case TimeUnit::MILLI: if (ts % 1000 == 0) { - return timestamp(TimeUnit::SECOND); + return timestamp(TimeUnit::SECOND, ts_type->timezone()); } return value.type(); case TimeUnit::MICRO: if (ts % 1000000 == 0) { - return timestamp(TimeUnit::SECOND); + return timestamp(TimeUnit::SECOND, ts_type->timezone()); } if (ts % 1000 == 0) { - return timestamp(TimeUnit::MILLI); + return timestamp(TimeUnit::MILLI, ts_type->timezone()); } return value.type(); case TimeUnit::NANO: if (ts % 1000000000 == 0) { - return timestamp(TimeUnit::SECOND); + return timestamp(TimeUnit::SECOND, ts_type->timezone()); } if (ts % 1000000 == 0) { - return timestamp(TimeUnit::MILLI); + return timestamp(TimeUnit::MILLI, ts_type->timezone()); } if (ts % 1000 == 0) { - return timestamp(TimeUnit::MICRO); + return timestamp(TimeUnit::MICRO, ts_type->timezone()); } return value.type(); default: diff --git a/cpp/src/arrow/compute/expression_test.cc b/cpp/src/arrow/compute/expression_test.cc index f90e01a2f81bc..b852f6f6b0cdb 100644 --- a/cpp/src/arrow/compute/expression_test.cc +++ b/cpp/src/arrow/compute/expression_test.cc @@ -63,6 +63,7 @@ const std::shared_ptr kBoringSchema = schema({ field("ts_ns", timestamp(TimeUnit::NANO)), field("ts_s", timestamp(TimeUnit::SECOND)), field("binary", binary()), + field("ts_s_utc", timestamp(TimeUnit::SECOND, "UTC")), }); #define EXPECT_OK ARROW_EXPECT_OK @@ -630,6 +631,18 @@ TEST(Expression, BindWithImplicitCasts) { literal(std::make_shared(0, TimeUnit::NANO))), cmp(field_ref("ts_s"), literal(std::make_shared(0, TimeUnit::SECOND)))); + // GH-37110 + ExpectBindsTo( + cmp(field_ref("ts_s_utc"), + literal(std::make_shared(0, TimeUnit::NANO, "UTC"))), + cmp(field_ref("ts_s_utc"), + literal(std::make_shared(0, TimeUnit::SECOND, "UTC")))); + ExpectBindsTo( + cmp(field_ref("ts_s_utc"), + literal(std::make_shared(123000, TimeUnit::NANO, "UTC"))), + cmp(field_ref("ts_s_utc"), + literal(std::make_shared(123, TimeUnit::MICRO, "UTC")))); + ExpectBindsTo( cmp(field_ref("binary"), literal(std::make_shared("foo"))), cmp(field_ref("binary"), literal(std::make_shared("foo"))));