finished adding type aware parser for time and timstamp#37965
Open
manasagar wants to merge 1 commit intoapache:masterfrom
Open
finished adding type aware parser for time and timstamp#37965manasagar wants to merge 1 commit intoapache:masterfrom
manasagar wants to merge 1 commit intoapache:masterfrom
Conversation
terrymanu
requested changes
Feb 6, 2026
Member
terrymanu
left a comment
There was a problem hiding this comment.
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:
- In PostgreSQLDataRowPacket.writeTextValue, format timestamp/timestamptz/time/timetz per PostgreSQL text protocol: max 6 fractional digits, trim trailing zeros, preserve session time zone.
- Base formatting on column type, not only runtime instanceof, so Timestamp values are handled.
- 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #37437
Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.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
TIME'10:00:00'10:00:00TIME'10:00:00.0'10:00:00TIME'10:00:00.1'10:00:00.1TIMETZ'10:00:00+05:30'10:00:00+05:30TIMETZ'10:00:00.1+05:30'10:00:00.100000+05:30TIMETZ'10:00:00.0'10:00:00TIMESTAMP'2026-02-04 10:00:00'2026-02-04 10:00:00TIMESTAMP'2026-02-04 10:00:00.0'2026-02-04 10:00:00TIMESTAMP'2026-02-04 10:00:00.1'2026-02-04 10:00:00.1TIMESTAMPTZ'2026-02-04 10:00:00.0+05:30'2026-02-04 04:30:00+00TIMESTAMPTZ'2026-02-04 10:00:00.1+05:30'2026-02-04 04:30:00.1+00Key Points:
.0fractional seconds are truncatedTIMETZpads fractional seconds to 6 digitsTIMESTAMPTZconverts to UTC when timezone is specified