Arrow Connector : Preserve TIME and TIMESTAMP as Wall-Clock (Align with JDBC Connector) by lithinwxd · Pull Request #25901 · prestodb/presto

lithinwxd · 2025-08-27T15:21:51Z

Description

This PR fixes the handling of TIME and TIMESTAMP values in the Arrow connector.
Previously, Arrow interpreted raw millis as UTC instants, which caused unintended timezone shifts when queried from Presto. With this change, the Arrow connector preserves the values as wall-clock times, consistent with the behavior of the JDBC connector.

Example
A source value of 2025-08-01 10:00:00 (TIMESTAMP) was treated as 2025-08-01 10:00 UTC, which then shifted when displayed in Asia/Kolkata or America/Los_Angeles.

Similarly, TIME '10:00:00' was treated as 10:00 UTC instead of 10:00 wall-clock.

Motivation and Context

Align Arrow connector semantics with the JDBC connector for TIME and TIMESTAMP.
Ensure that values from data sources without timezone information are not shifted incorrectly.
Improves user experience by showing the same results across connectors.

Impact

User-facing change: TIME and TIMESTAMP values will now be preserved as stored (wall-clock).
This eliminates double timezone offset shifts.
No impact on other connectors or types.
No performance regression is expected, since the change only affects value interpretation.

Test Plan

Updated unit tests in TestArrowBlockBuilder to validate correct preservation of TIME and TIMESTAMP.
Verified results against JDBC connector outputs to confirm alignment.
Ran queries in sessions with different time zones to confirm consistent wall-clock behavior.

Contributor checklist

Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Fix Arrow connector handling of TIME and TIMESTAMP types.
  These values are now preserved as wall-clock times (without implicit UTC interpretation),
  consistent with the JDBC connector.

...arrow-flight/src/test/java/com/facebook/plugin/arrow/testingServer/TestingArrowProducer.java

presto-base-arrow-flight/src/main/java/com/facebook/plugin/arrow/ArrowBlockBuilder.java

tdcmeehan · 2025-08-28T15:08:52Z

Do you see this problem when you toggle the legacy_timestamp property?

lithinwxd · 2025-08-29T11:28:48Z

legacy_timestamp

@tdcmeehan I see the issue even if i set the property SET SESSION legacy_timestamp = true;

imjalpreet · 2025-09-17T10:17:18Z

I see the issue even if i set the property SET SESSION legacy_timestamp = true;

@lithinwxd, true is the default value. Did you also try to set it to false?

lithinwxd · 2025-09-18T06:30:47Z

SET SESSION legacy_timestamp = true;

@imjalpreet I tried with false as well , same problem exists without the fix . After the fix , I see consistent results with jdbc and arrow . I tried testing with client as (Moscow UTC +3) and the results got rendered for client according to their timezone.

lithinwxd · 2025-09-24T15:51:05Z

@imjalpreet here is the analysis for a sample Query in oracle for timestamp type

query : SELECT customer_id , created_at FROM tm_lakehouse_gfs_user.customer_master cm;

value in datasource : 2023-01-01 10:15:30.000

⏱ Timestamp Behavior Comparison

Legacy Timestamp = false

Tool	customer_id	created_at
opensource presto	1	2023-01-01 04:45:30.000
Arrow Flight (before fix)	1	2023-01-01 04:45:30.000
Arrow Flight (after fix)	1	2023-01-01 04:45:30.000
DBeaver	1	2023-01-01 10:15:30.000
DBeaver( after setting custom timezone)	1	2023-01-01 10:15:30.000

Legacy Timestamp = true

Tool	customer_id	created_at
opensource presto	1	2023-01-01 10:15:30.000
Arrow Flight (before fix)	1	2023-01-01 15:45:30.000
Arrow Flight (after fix)	1	2023-01-01 10:15:30.000
DBeaver	1	2023-01-01 15:45:30.000
DBeaver( after setting custom timezone)	1	2023-01-01 10:15:30.000

note : All queries are executed wrt to IST timezone(UTC+5h30min)

BryanCutler

Thanks @lithinwxd , but I don't think this is the right approach. An Arrow TimeMilliVector is not meant to be UTC and Arrow does not make any interpretation of values to be in UTC, see https://github.com/apache/arrow/blob/main/format/Schema.fbs#L307.

Presto has been interpreting these values as local time and we should continue to do this. If the data source is providing values in UTC, then it should be using a TimeStampMilliTZVector and set the time zone to UTC.

We should fix handling of explicit time zones and leave vectors with no time zone to be local time, not assume UTC. Then whatever data source is producing your values, they should either have a timezone or be in local time before writing to the Arrow vector.

BryanCutler · 2025-09-29T16:43:11Z

presto-base-arrow-flight/src/main/java/com/facebook/plugin/arrow/ArrowBlockBuilder.java

@@ -432,11 +436,25 @@ public void assignBlockFromTimeStampMilliVector(TimeStampMilliVector vector, Typ
            }


What about assignBlockFromTimeStampMicroVector ?

BryanCutler · 2025-09-29T16:46:05Z

presto-base-arrow-flight/src/main/java/com/facebook/plugin/arrow/ArrowBlockBuilder.java

@@ -639,7 +659,9 @@ public void assignBlockFromTimeMilliTZVector(TimeStampMilliTZVector vector, Type
            }


A TimeStampMilliTZVector has a specific TimeZone, you would need to use this instead of treating it as UTC. Actually, I think we should be converting to the presto data type TimestampWithTimeZoneType and use the TimeZone from the Arrow vector. Can you try this?

BryanCutler · 2025-09-29T16:48:25Z

presto-base-arrow-flight/src/test/java/com/facebook/plugin/arrow/TestArrowBlockBuilder.java

+                    .atZone(ZoneOffset.UTC)   // interpret Arrow millis as UTC
+                    .toLocalDateTime();
+
+            value = localDateTime


nit: don't reuse value, make a new variable expected

BryanCutler · 2025-09-29T17:31:10Z

presto-base-arrow-flight/src/test/java/com/facebook/plugin/arrow/TestArrowBlockBuilder.java

+                    .atZone(ZoneOffset.UTC)   // interpret Arrow millis as UTC
+                    .toLocalDateTime();
+
+            expectedMillis = localDateTime


nit: change the definition of long expectedMillis to just long value and use expectedMillis here

BryanCutler · 2025-09-29T17:46:00Z

presto-base-arrow-flight/src/main/java/com/facebook/plugin/arrow/ArrowBlockBuilder.java

                long millis = vector.get(i);
-                type.writeLong(builder, millis);
+                // Interpret Arrow millis as if they were "local wall-clock time" in datasource
+                long localMillis = getLocalMillisFromUTCMillis(millis);


A TimeMilliVector has no associated timezone, but here you are forcing it to be a UTC timezone, we shouldn't do this. We have to assume that a timestamp without a timezone is local time.

BryanCutler · 2025-09-29T17:52:30Z

...arrow-flight/src/test/java/com/facebook/plugin/arrow/testingServer/TestingArrowProducer.java

            try (ResultSet resultSet = stmt.executeQuery(query.toUpperCase())) {
                JdbcToArrowConfig config = new JdbcToArrowConfigBuilder().setAllocator(allocator).setTargetBatchSize(2048)
-                        .setCalendar(Calendar.getInstance(TimeZone.getDefault())).build();
+                        .setCalendar(Calendar.getInstance(TimeZone.getTimeZone("UTC"))).build();


Yes, your assumption is correct. Timestamp vectors are represented as UTC instants in Arrow since arrow doesn't have timezone.

This is wrong, an Arrow TimeStampMilliVector does not assume UTC. It's interpretation is up to presto, and we have been using this as local time. You must use a TimeStampMilliTZVector and set the time zone to UTC, if you want a UTC values.

You shouldn't need to change this in the test, it is saying that our datasource is generating localtime data with a timezone, resulting in a TimeStampMilliTZVector with the system timezone.

lithinwxd · 2025-09-30T11:41:31Z

@BryanCutler I did some testing comparing the jdbc values vs arrow values and here is the result

The value obtained from jdbc oracle connector is 1672548330000 ms

The value obtained from arrow based oracle connector without fix is 1672568130000 ms (TimeStampMilliVector)

I checked the unix timestamp calculator for both of these values and here is the info found

jdbc

Arrow

Note : this was tested for a timestamp column and not for Timestamp with timezone column

BryanCutler · 2025-09-30T17:37:00Z

I did some testing comparing the jdbc values vs arrow values and here is the result

For an Arrow TimeStampMilliVector the value is just a long, there are no conversions. You need to make sure whatever values produced in your Arrow Flight server are correct, not adjust Presto to fit your server. Values without timezones should not assume to have a UTC timezone, they can only be interpreted as time since epoch relative to the local system time.

Please read the Arrow format spec, it is very clear https://github.com/apache/arrow/blob/main/format/Schema.fbs#L304

lithinwxd · 2025-10-01T06:58:18Z

I did some testing comparing the jdbc values vs arrow values and here is the result

For an Arrow TimeStampMilliVector the value is just a long, there are no conversions. You need to make sure whatever values produced in your Arrow Flight server are correct, not adjust Presto to fit your server. Values without timezones should not assume to have a UTC timezone, they can only be interpreted as time since epoch relative to the local system time.

Please read the Arrow format spec, it is very clear https://github.com/apache/arrow/blob/main/format/Schema.fbs#L304

From arrow server side perspective , if we are not using a UTC instant , instead if we use local system time that would default to arrow server time which could be anywhere say here (UTC-7) its present in a vm, then there would be double shifts . Datasource can be in a cloud but vm may be in UTC-7 zone . If you must use an Arrow timestamp vector ( from server side) like TimeStampMilliVector, you have no choice but to convert the local datetime to epoch millis using some timezone (e.g., system default or UTC), and timezone-based shifts will happen.

BryanCutler · 2025-10-01T16:35:46Z

local system time that would default to arrow server time which could be anywhere

That is the problem, you are changing the data in Presto irreversibly by converting it to the system timezone. You are free to change the data in your flight server, but Presto should not be manipulating the data this way.

It would be best if you can make a new unit test that will fail without your changes, but pass with your changes applied. Once you have this, it will be easier to discuss. Can you do that?

lithinwxd · 2025-10-03T15:59:51Z

local system time that would default to arrow server time which could be anywhere

That is the problem, you are changing the data in Presto irreversibly by converting it to the system timezone. You are free to change the data in your flight server, but Presto should not be manipulating the data this way.

It would be best if you can make a new unit test that will fail without your changes, but pass with your changes applied. Once you have this, it will be easier to discuss. Can you do that?

@BryanCutler I tried creating a new unit test to replicate the issue experienced in Arrow server but it seems even the existing test cases in master fails when the Presto client and Arrow server are in different timezone. We are not trying to irreversibly change the data may be until legacy timestamp issue is fixed this fix might solve the issue, we need to make sure both jdbc and arrow replicate same values. One thing I observed is arrow is returning same millis but presto according to whether legacy timestamp is set or not is reintrepreting the result. Until a fix for legacy timestamp is in place this issue cannot be resolved.

lithinwxd · 2025-10-31T07:30:56Z

#26449 - shall close once legacy timestamp issue is merged

tdcmeehan · 2025-11-12T14:16:13Z

Closing as this was trying to match a bug in the JDBC connectors, which was fixed in #26449.

lithinwxd added 2 commits August 27, 2025 20:40

Fix Time and Timestamp and time zone conversion issues for Arrow

c631eab

Fix Time and Timestamp and time zone conversion issues for Arrow

8159a3d

lithinwxd requested a review from a team as a code owner August 27, 2025 15:21

lithinwxd marked this pull request as draft August 27, 2025 15:22

lithinwxd changed the title ~~Feature timestamp fix for arrow~~ Arrow Connector : Preserve TIME and TIMESTAMP as Wall-Clock (Align with JDBC Connector) Aug 27, 2025

lithinwxd marked this pull request as ready for review August 28, 2025 04:28

Merge branch 'prestodb:master' into feature_timestampFixForArrow

c5f4adb

elbinpallimalilibm reviewed Aug 28, 2025

View reviewed changes

...arrow-flight/src/test/java/com/facebook/plugin/arrow/testingServer/TestingArrowProducer.java Show resolved Hide resolved

elbinpallimalilibm reviewed Aug 28, 2025

View reviewed changes

presto-base-arrow-flight/src/main/java/com/facebook/plugin/arrow/ArrowBlockBuilder.java Show resolved Hide resolved

tdcmeehan self-assigned this Aug 28, 2025

tdcmeehan requested a review from imjalpreet September 8, 2025 14:24

Merge branch 'prestodb:master' into feature_timestampFixForArrow

4415b48

lithinwxd added 2 commits September 26, 2025 14:49

Merge branch 'prestodb:master' into feature_timestampFixForArrow

d290d44

Merge branch 'prestodb:master' into feature_timestampFixForArrow

af5f46e

ethanyzhang requested a review from BryanCutler September 29, 2025 15:21

BryanCutler requested changes Sep 29, 2025

View reviewed changes

BryanCutler mentioned this pull request Oct 17, 2025

test(plugin-arrow): Adding Arrow tests for date, time and timestamp vectors #26357

Merged

7 tasks

tdcmeehan closed this Nov 12, 2025

		@@ -432,11 +436,25 @@ public void assignBlockFromTimeStampMilliVector(TimeStampMilliVector vector, Typ
		}

		@@ -639,7 +659,9 @@ public void assignBlockFromTimeMilliTZVector(TimeStampMilliTZVector vector, Type
		}

Conversation

lithinwxd commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

Uh oh!

Uh oh!

Uh oh!

tdcmeehan commented Aug 28, 2025

Uh oh!

lithinwxd commented Aug 29, 2025

Uh oh!

imjalpreet commented Sep 17, 2025

Uh oh!

lithinwxd commented Sep 18, 2025

Uh oh!

lithinwxd commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏱ Timestamp Behavior Comparison

Legacy Timestamp = false

Legacy Timestamp = true

Uh oh!

BryanCutler left a comment

Choose a reason for hiding this comment

Uh oh!

BryanCutler Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

BryanCutler Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

BryanCutler Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

BryanCutler Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

BryanCutler Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

BryanCutler Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

lithinwxd commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BryanCutler commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lithinwxd commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BryanCutler commented Oct 1, 2025

Uh oh!

lithinwxd commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lithinwxd commented Oct 31, 2025

Uh oh!

tdcmeehan commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lithinwxd commented Aug 27, 2025 •

edited

Loading

lithinwxd commented Sep 24, 2025 •

edited

Loading

lithinwxd commented Sep 30, 2025 •

edited

Loading

BryanCutler commented Sep 30, 2025 •

edited

Loading

lithinwxd commented Oct 1, 2025 •

edited

Loading

lithinwxd commented Oct 3, 2025 •

edited

Loading