Skip to content

Fix timestamp-to-date conversion - #7820

Merged
mattcasters merged 5 commits into
apache:mainfrom
gvdutra:5651-convert-timestamp-to-date
Oct 9, 2026
Merged

mattcasters merged 5 commits into
apache:mainfrom
gvdutra:5651-convert-timestamp-to-date

Conversation

@gvdutra

@gvdutra gvdutra commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • return a plain java.util.Date when converting timestamp data to Date metadata
  • keep timestamp access and JDBC timestamp writes unchanged
  • add regression coverage for conversion, null handling, and JDBC writes

Root cause

Date conversion delegated directly to the source metadata. For timestamp fields, that returned the original java.sql.Timestamp, so the field metadata changed to Date while the runtime value did not.

The conversion is now normalized only when the destination type is Date, avoiding changes to timestamp consumers and preserving timestamp precision in existing database paths.

Validation

  • ./mvnw -pl core -Dtest=ValueMetaTimestampTest test
  • ./mvnw -pl core test
  • ./mvnw -pl core -DskipTests spotless:check
  • git diff --check

Closes #5651

@gvdutra
gvdutra marked this pull request as ready for review August 8, 2026 03:48
@hansva
hansva marked this pull request as draft August 10, 2026 06:58
@hansva

hansva commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Lifting this to 2.20

@hansva hansva added this to the 2.20 milestone Aug 10, 2026
@hansva

hansva commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Shouldn't this be fixed in the getDate function?

convertData already copied a Timestamp or java.sql.Date to a plain
java.util.Date when the destination type is Date. convertDataCompatible,
used when Stream Lookup coerces a key, still returned the source value.
Both paths now share that copy. Timestamp fields still return the
original Timestamp, nanoseconds included.
@mattcasters

Copy link
Copy Markdown
Contributor

Analysis of where a Timestamp becomes a Date, for #5651.

java.sql.Timestamp extends java.util.Date, and ValueMetaTimestamp.getDate() returns that same instance. The JDBC specification describes this as implementation inheritance: code should not view a Timestamp as a Date. The same note applies to java.sql.Date.

Two Hop paths meet that rule in different places.

Reading a field whose metadata is still Timestamp. getDate() is the accessor for that field. ValueMetaTimestamp.setPreparedStatementValue writes through getTimestamp(), so a timestamp column keeps its nanoseconds. OraBulkDataOutput casts rowMeta.getDate(...) to Timestamp for a timestamp column. Returning new Date(timestamp.getTime()) from getDate() would make that cast fail, and every caller that asks a timestamp field for a Date would lose the nanoseconds.

Converting into a field whose metadata is Date. Select Values does this with toMeta.convertData(fromMeta, value) when the type changes. Stream Lookup coerces a key with convertDataCompatible when the lookup type differs from the input. Both methods returned the source getDate() result unchanged, so a Timestamp-to-Date conversion changed the metadata and left a Timestamp in the row. JdbcDateValues.write then sees instanceof Timestamp and calls setTimestamp with the original nanoseconds. That is the conversion failure in #5651.

getValueFromResultSet can also store ResultSet.getTimestamp() in a Date field when the dialect supports timestamp-to-date conversion. Those rows are already Date metadata, so they do not go through ValueMetaTimestamp.getDate(). JdbcDateValues writes the stored object and does not call getDate(). Whether a Date field may still carry a Timestamp after a read is separate from this conversion bug. This pull request leaves that read path and JdbcDateValues as they are.

The normalization is in the two conversion methods, and only when the destination type is Date. A Timestamp or java.sql.Date is copied with new Date(date.getTime()). A value that is already a plain java.util.Date is returned as the same instance. ValueMetaTimestamp.getDate() and ValueMetaTimestamp.convertData() are unchanged, so a timestamp field still converts as a Timestamp and keeps its nanoseconds.

The branch now contains current main. The additional commit applies the same Date normalization to convertDataCompatible. The first patch covered convertData only.

@mattcasters
mattcasters marked this pull request as ready for review October 1, 2026 19:14
@mattcasters
mattcasters merged commit f6c0adc into apache:main Oct 9, 2026
3 checks passed
@github-actions github-actions Bot removed this from the 2.20 milestone Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Converting a Timestamp to a Date doesn't do anything

3 participants