Skip to content

Commit

Permalink
apacheGH-37110: [C++] Expression: SmallestTypeFor lost tz for Scalar (a…
Browse files Browse the repository at this point in the history
…pache#37135)

### Rationale for this change

This patch ( apache#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: apache#37110

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
mapleFU authored and loicalleyne committed Nov 13, 2023
1 parent 7489656 commit 6fa4752
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
12 changes: 6 additions & 6 deletions cpp/src/arrow/compute/expression.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 13 additions & 0 deletions cpp/src/arrow/compute/expression_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const std::shared_ptr<Schema> 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
Expand Down Expand Up @@ -630,6 +631,18 @@ TEST(Expression, BindWithImplicitCasts) {
literal(std::make_shared<TimestampScalar>(0, TimeUnit::NANO))),
cmp(field_ref("ts_s"),
literal(std::make_shared<TimestampScalar>(0, TimeUnit::SECOND))));
// GH-37110
ExpectBindsTo(
cmp(field_ref("ts_s_utc"),
literal(std::make_shared<TimestampScalar>(0, TimeUnit::NANO, "UTC"))),
cmp(field_ref("ts_s_utc"),
literal(std::make_shared<TimestampScalar>(0, TimeUnit::SECOND, "UTC"))));
ExpectBindsTo(
cmp(field_ref("ts_s_utc"),
literal(std::make_shared<TimestampScalar>(123000, TimeUnit::NANO, "UTC"))),
cmp(field_ref("ts_s_utc"),
literal(std::make_shared<TimestampScalar>(123, TimeUnit::MICRO, "UTC"))));

ExpectBindsTo(
cmp(field_ref("binary"), literal(std::make_shared<LargeBinaryScalar>("foo"))),
cmp(field_ref("binary"), literal(std::make_shared<BinaryScalar>("foo"))));
Expand Down

0 comments on commit 6fa4752

Please sign in to comment.