Skip to content

GH-49454: [C++][Gandiva] Fix castVARCHAR_timestamp for pre-epoch timestamps#49455

Merged
kou merged 1 commit intoapache:mainfrom
dmitry-chirkov-dremio:fix-castVARCHAR-timestamp-negative-millis
Mar 13, 2026
Merged

GH-49454: [C++][Gandiva] Fix castVARCHAR_timestamp for pre-epoch timestamps#49455
kou merged 1 commit intoapache:mainfrom
dmitry-chirkov-dremio:fix-castVARCHAR-timestamp-negative-millis

Conversation

@dmitry-chirkov-dremio
Copy link
Contributor

@dmitry-chirkov-dremio dmitry-chirkov-dremio commented Mar 5, 2026

Rationale for this change

GH-49454 castVARCHAR_timestamp_int64 produces negative milliseconds for pre-epoch timestamps

What changes are included in this PR?

Fixed castVARCHAR_timestamp_int64 to correctly handle pre-epoch timestamps (before 1970-01-01). The issue was that using in % MILLIS_IN_SEC on negative timestamps produces negative milliseconds, resulting in output like "0107-10-17 12:20:03.-10".

Are these changes tested?

Yes, added 4 new test cases covering pre-epoch timestamps with milliseconds

Are there any user-facing changes?

This PR contains a "Critical Fix".
This fixes a bug that caused incorrect data to be produced when casting pre-epoch timestamps to VARCHAR in Gandiva. Previously, timestamps before 1970-01-01 with non-zero milliseconds would produce invalid output with negative millisecond values (e.g., "0107-10-17 12:20:03.-10" instead of "0107-10-17 12:20:03.900").

@dmitry-chirkov-dremio dmitry-chirkov-dremio force-pushed the fix-castVARCHAR-timestamp-negative-millis branch from b4b0c4c to 9e9ce58 Compare March 5, 2026 03:47
Copy link
Contributor

@akravchukdremio akravchukdremio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dmitry-chirkov-dremio
Copy link
Contributor Author

@kou Think this needs awaiting committer review label

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 13, 2026
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@kou kou merged commit d436b23 into apache:main Mar 13, 2026
67 of 75 checks passed
@kou kou removed the awaiting committer review Awaiting committer review label Mar 13, 2026
@github-actions github-actions bot added the awaiting merge Awaiting merge label Mar 13, 2026
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit d436b23.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 11 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants