-
-
Notifications
You must be signed in to change notification settings - Fork 46
fix: EXPLAIN ANALYZE fails with date/time field value out of range for binary-encoded timestamp args
#24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
fix: EXPLAIN ANALYZE fails with date/time field value out of range for binary-encoded timestamp args
#24
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
135e751
Initial plan
Copilot 5489291
fix: convert binary-encoded timestamp args to time.Time in explain Run()
Copilot 67deaca
fix lint: use explain_test package and apply gofmt to build_args_test.go
Copilot ad49ce2
fix: use Parse ParameterOIDs to decode binary timestamp args as RFC33…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,236 @@ | ||
| package explain_test | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/mickamy/sql-tap/explain" | ||
| ) | ||
|
|
||
| func TestParseTimestampParams(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| query string | ||
| want map[int]bool | ||
| }{ | ||
| { | ||
| name: "no casts", | ||
| query: "SELECT * FROM users WHERE id = $1", | ||
| want: map[int]bool{}, | ||
| }, | ||
| { | ||
| name: "timestamp with time zone", | ||
| query: "SELECT * FROM t WHERE created_at > $1::TIMESTAMP WITH TIME ZONE", | ||
| want: map[int]bool{1: true}, | ||
| }, | ||
| { | ||
| name: "timestamptz", | ||
| query: "SELECT * FROM t WHERE ts = $2::TIMESTAMPTZ", | ||
| want: map[int]bool{2: true}, | ||
| }, | ||
| { | ||
| name: "timestamp without time zone", | ||
| query: "SELECT * FROM t WHERE ts = $3::TIMESTAMP WITHOUT TIME ZONE", | ||
| want: map[int]bool{3: true}, | ||
| }, | ||
| { | ||
| name: "plain timestamp", | ||
| query: "SELECT * FROM t WHERE ts = $1::TIMESTAMP", | ||
| want: map[int]bool{1: true}, | ||
| }, | ||
| { | ||
| name: "lowercase cast", | ||
| query: "SELECT * FROM t WHERE ts > $1::timestamp with time zone", | ||
| want: map[int]bool{1: true}, | ||
| }, | ||
| { | ||
| name: "spaces around ::", | ||
| query: "SELECT * FROM t WHERE ts = $1 :: TIMESTAMP", | ||
| want: map[int]bool{1: true}, | ||
| }, | ||
| { | ||
| name: "mixed: timestamp and non-timestamp", | ||
| query: "SELECT * FROM t WHERE key = $1::VARCHAR AND ts > $2::TIMESTAMP WITH TIME ZONE", | ||
| want: map[int]bool{2: true}, | ||
| }, | ||
| { | ||
| name: "multiple timestamp params", | ||
| query: "SELECT * FROM t WHERE a > $1::TIMESTAMP AND b < $3::TIMESTAMPTZ", | ||
| want: map[int]bool{1: true, 3: true}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| got := explain.ParseTimestampParams(tt.query) | ||
| if len(got) != len(tt.want) { | ||
| t.Fatalf("ParseTimestampParams(%q) = %v, want %v", tt.query, got, tt.want) | ||
| } | ||
| for k, v := range tt.want { | ||
| if got[k] != v { | ||
| t.Errorf("ParseTimestampParams(%q)[%d] = %v, want %v", tt.query, k, got[k], v) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestParsePGTimestamp(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| input string | ||
| want time.Time | ||
| wantOK bool | ||
| }{ | ||
| { | ||
| name: "PostgreSQL epoch (zero)", | ||
| input: "0", | ||
| want: time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC), | ||
| wantOK: true, | ||
| }, | ||
| { | ||
| name: "large microseconds value (issue repro: ~2026)", | ||
| input: "825505830505628", | ||
| want: func() time.Time { | ||
| microsecs := int64(825505830505628) | ||
| sec := microsecs / 1_000_000 | ||
| usec := microsecs % 1_000_000 | ||
| return time.Unix(sec+explain.PgEpochUnix, usec*1_000).UTC() | ||
| }(), | ||
| wantOK: true, | ||
| }, | ||
| { | ||
| name: "negative (before 2000-01-01)", | ||
| input: "-1000000", | ||
| want: time.Date(1999, 12, 31, 23, 59, 59, 0, time.UTC), | ||
| wantOK: true, | ||
| }, | ||
| { | ||
| name: "negative fractional (before 2000-01-01)", | ||
| input: "-1", | ||
| want: time.Date(1999, 12, 31, 23, 59, 59, 999999000, time.UTC), | ||
| wantOK: true, | ||
| }, | ||
| { | ||
| name: "non-integer string", | ||
| input: "2026-02-27T14:10:30Z", | ||
| wantOK: false, | ||
| }, | ||
| { | ||
| name: "float string", | ||
| input: "1.5", | ||
| wantOK: false, | ||
| }, | ||
| { | ||
| name: "empty string", | ||
| input: "", | ||
| wantOK: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| got, ok := explain.ParsePGTimestamp(tt.input) | ||
| if ok != tt.wantOK { | ||
| t.Fatalf("ParsePGTimestamp(%q) ok = %v, want %v", tt.input, ok, tt.wantOK) | ||
| } | ||
| if ok && !got.Equal(tt.want) { | ||
| t.Errorf("ParsePGTimestamp(%q) = %v, want %v", tt.input, got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestBuildAnyArgs(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| pgEpoch := time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC) | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| query string | ||
| args []string | ||
| check func(t *testing.T, got []any) | ||
| }{ | ||
| { | ||
| name: "no args", | ||
| query: "SELECT 1", | ||
| args: nil, | ||
| check: func(t *testing.T, got []any) { | ||
| t.Helper() | ||
| if len(got) != 0 { | ||
| t.Errorf("expected empty slice, got %v", got) | ||
| } | ||
| }, | ||
| }, | ||
| { | ||
| name: "non-timestamp arg stays as string", | ||
| query: "SELECT * FROM users WHERE id = $1", | ||
| args: []string{"42"}, | ||
| check: func(t *testing.T, got []any) { | ||
| t.Helper() | ||
| if s, ok := got[0].(string); !ok || s != "42" { | ||
| t.Errorf("got[0] = %v (%T), want string %q", got[0], got[0], "42") | ||
| } | ||
| }, | ||
| }, | ||
| { | ||
| name: "timestamp arg converted to time.Time", | ||
| query: "SELECT * FROM t WHERE expired_at > $1::TIMESTAMP WITH TIME ZONE", | ||
| args: []string{"825505830505628"}, | ||
| check: func(t *testing.T, got []any) { | ||
| t.Helper() | ||
| ts, ok := got[0].(time.Time) | ||
| if !ok { | ||
| t.Fatalf("got[0] = %v (%T), want time.Time", got[0], got[0]) | ||
| } | ||
| // The value should be ~2026 (pgEpoch + 825505830.5 seconds) | ||
| if ts.Before(pgEpoch) { | ||
| t.Errorf("got time %v, expected a time after 2000-01-01", ts) | ||
| } | ||
| }, | ||
| }, | ||
| { | ||
| name: "non-integer timestamp arg stays as string", | ||
| query: "SELECT * FROM t WHERE ts > $1::TIMESTAMP WITH TIME ZONE", | ||
| args: []string{"2026-02-27T14:10:30Z"}, | ||
| check: func(t *testing.T, got []any) { | ||
| t.Helper() | ||
| if s, ok := got[0].(string); !ok || s != "2026-02-27T14:10:30Z" { | ||
| t.Errorf("got[0] = %v (%T), want string %q", got[0], got[0], "2026-02-27T14:10:30Z") | ||
| } | ||
| }, | ||
| }, | ||
| { | ||
| name: "mixed args: varchar and timestamp", | ||
| query: "SELECT * FROM t WHERE key = $1::VARCHAR AND ts > $2::TIMESTAMP WITH TIME ZONE", | ||
| args: []string{"019c5c4f-f25a-772b-97d4-1646a125080d", "825505830505628"}, | ||
| check: func(t *testing.T, got []any) { | ||
| t.Helper() | ||
| if s, ok := got[0].(string); !ok || s != "019c5c4f-f25a-772b-97d4-1646a125080d" { | ||
| t.Errorf("got[0] = %v (%T), want string", got[0], got[0]) | ||
| } | ||
| if _, ok := got[1].(time.Time); !ok { | ||
| t.Errorf("got[1] = %v (%T), want time.Time", got[1], got[1]) | ||
| } | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| got := explain.BuildAnyArgs(tt.query, tt.args) | ||
| tt.check(t, got) | ||
| }) | ||
| } | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package explain | ||
|
|
||
| // Exported wrappers for internal symbols used in package-external tests. | ||
|
|
||
| var ( | ||
| BuildAnyArgs = buildAnyArgs | ||
| ParseTimestampParams = parseTimestampParams | ||
| ParsePGTimestamp = parsePGTimestamp | ||
| ) | ||
|
|
||
| const PgEpochUnix = pgEpochUnix |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildAnyArgs() only converts integer args when the query text contains an explicit
$N::TIMESTAMP...cast. However, in the Postgres proxy the captured query used for EXPLAIN is the raw Parse query (q := c.lastParse) and typically does not include explicit casts; parameter types are usually inferred from context (e.g.,created_at > $1). In those cases this conversion will never trigger and EXPLAIN can still fail with SQLSTATE 22008 for binary-encoded timestamptz args. Consider using protocol type information instead of query-text regex (e.g., captureParse.ParameterOIDsand/or per-parameter format codes in the event / ExplainRequest) so timestamp parameters can be detected even when there is no::TIMESTAMPin the SQL.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot fix this issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in ad49ce2 by addressing the issue at the protocol level instead of relying on query-text regex.
Changes in
proxy/postgres/conn.go:handleParsenow capturesm.ParameterOIDsintolastParamOIDs(andpreparedStmtOIDsfor named statements)handleBindlooks up OIDs per parameter and passes them todecodeBinaryParamdecodeBinaryParamnow detects timestamp OIDs (1114 =timestamp, 1184 =timestamptz) and decodes the 8-byte binary value usingdecodePGTimestampMicros→ stores an RFC3339Nano string (e.g."2026-02-27T14:10:30.505628Z") that PostgreSQL can parse back correctly in both explicit-cast and implicit-cast contextsThe existing
buildAnyArgsregex inexplain.gois kept as a fallback for drivers that don't send OIDs but have explicit$N::TIMESTAMPcasts.