Skip to content

finished adding type aware parser for time and timstamp#37965

Open
manasagar wants to merge 1 commit intoapache:masterfrom
manasagar:37437
Open

finished adding type aware parser for time and timstamp#37965
manasagar wants to merge 1 commit intoapache:masterfrom
manasagar:37437

Conversation

@manasagar
Copy link
Contributor

Fixes #37437

Before committing this PR, I'm sure that I have checked the following options:

  • My code follows the code of conduct of this project.
  • I have self-reviewed the commit code.
  • I have (or in comment I request) added corresponding labels for the pull request.
  • I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
  • I have made corresponding changes to the documentation.
  • I have added corresponding unit tests for my changes.
  • I have updated the Release Notes of the current development version. For more details, see Update Release Note

Changes proposed in this pull request:

In the PostgreSQL text protocol write path (PostgreSQLDataRowPacket#writeTextValue), used type-aware formatting for timestamp/timestamptz/date/time instead of generic toString().

PostgreSQL 17 Temporal Data Behavior

Data Type Input Output
TIME '10:00:00' 10:00:00
TIME '10:00:00.0' 10:00:00
TIME '10:00:00.1' 10:00:00.1
TIMETZ '10:00:00+05:30' 10:00:00+05:30
TIMETZ '10:00:00.1+05:30' 10:00:00.100000+05:30
TIMETZ '10:00:00.0' 10:00:00
TIMESTAMP '2026-02-04 10:00:00' 2026-02-04 10:00:00
TIMESTAMP '2026-02-04 10:00:00.0' 2026-02-04 10:00:00
TIMESTAMP '2026-02-04 10:00:00.1' 2026-02-04 10:00:00.1
TIMESTAMPTZ '2026-02-04 10:00:00.0+05:30' 2026-02-04 04:30:00+00
TIMESTAMPTZ '2026-02-04 10:00:00.1+05:30' 2026-02-04 04:30:00.1+00

Key Points:

  • .0 fractional seconds are truncated
  • TIMETZ pads fractional seconds to 6 digits
  • TIMESTAMPTZ converts to UTC when timezone is specified

Copy link
Member

@terrymanu terrymanu left a comment

Choose a reason for hiding this comment

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

Thanks for the effort to make the text protocol type-aware and for adding initial time-related tests—this is the right direction.

Change requests:

  • Root cause still unresolved: the proxy returns java.sql.Timestamp, which still goes through each.toString() and keeps the .0 suffix. Issue #37437 remains.
  • Time zone semantics are broken: the OffsetDateTime branch forces withOffsetSameInstant(UTC) and emits +00, diverging from PostgreSQL’s session time zone output and risking visible offsets.
  • Trailing-zero handling is inconsistent: OffsetTime always pads to 6 decimals, so 10:00:00.0+05:30 becomes 10:00:00.000000+05:30; LocalTime/LocalDateTime precision and trimming still don’t align with PostgreSQL rules.
  • Type detection: relying on instanceof misses the main Timestamp path from JDBC. The formatter should pick behavior based on column type, not just runtime class.
  • Tests don’t cover the failing scenario (timestamp without fractional seconds) or timestamptz/timetz behavior, so the fix isn’t demonstrated and regressions aren’t guarded.
  • Please keep ResultSetUtils untouched for this issue; the correct place to fix is the PostgreSQL text protocol formatter, not the generic JDBC conversion utilities.

Recommended next steps:

  1. In PostgreSQLDataRowPacket.writeTextValue, format timestamp/timestamptz/time/timetz per PostgreSQL text protocol: max 6 fractional digits, trim trailing zeros, preserve session time zone.
  2. Base formatting on column type, not only runtime instanceof, so Timestamp values are handled.
  3. Add tests for the reported case and for timestamptz/timetz outputs to prove the fix.

Please address these before we merge; happy to review the next revision.

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.

PostgreSQL proxy: timestamp output adds trailing ".0" compared to native PostgreSQL

3 participants