Skip to content

Commit 1e26207

Browse files
mason-sharpclaude
andauthored
Fix timestamp/time type handling in diff and repair pipelines
* Fix timestamp/time type handling in diff and repair pipelines - Split combined timestamp/timestamptz case in ConvertToPgxType; return pgxv5type.Timestamp for "timestamp without time zone" so pgx sends the correct wire type - Add time/timetz support: serialize pgxv5type.Time in diff output, parse time strings back to pgxv5type.Time in ConvertToPgxType, and pass timetz as string (unregistered in pgx v5 type map) - Exclude "time with time zone" from IsKnownScalarType so the diff layer casts it to ::TEXT (avoids scan error on unregistered type) - Wrap time.Time in pgxv5type.Timestamp in repair's convertValueForType for timestamp-without-tz columns - Add unit tests for all new type handling paths - Add integration test exercising full diff→repair round-trip for all four temporal types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add bounds checks and fractional truncation to parseTimeToMicroseconds Reject out-of-range hours (>23), minutes (>59), and seconds (>59). Truncate fractional second digits beyond 6 to microsecond precision. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fb985b6 commit 1e26207

6 files changed

Lines changed: 757 additions & 8 deletions

File tree

internal/consistency/diff/table_diff.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/google/uuid"
3434
"github.com/jackc/pgtype"
3535
"github.com/jackc/pgx/v5"
36+
pgxv5type "github.com/jackc/pgx/v5/pgtype"
3637
"github.com/jackc/pgx/v5/pgxpool"
3738
"github.com/pgedge/ace/db/queries"
3839
auth "github.com/pgedge/ace/internal/infra/db"
@@ -624,6 +625,19 @@ func (t *TableDiffTask) fetchRows(nodeName string, r Range) ([]types.OrderedMap,
624625
// nil caught above
625626
processedVal = fmt.Sprintf("%x-%x-%x-%x-%x",
626627
v[0:4], v[4:6], v[6:8], v[8:10], v[10:16])
628+
case pgxv5type.Time: // pgx/v5 returns "time without time zone" as pgtype.Time
629+
if v.Valid {
630+
usec := v.Microseconds
631+
hours := usec / 3_600_000_000
632+
usec -= hours * 3_600_000_000
633+
minutes := usec / 60_000_000
634+
usec -= minutes * 60_000_000
635+
seconds := usec / 1_000_000
636+
usec -= seconds * 1_000_000
637+
processedVal = fmt.Sprintf("%02d:%02d:%02d.%06d", hours, minutes, seconds, usec)
638+
} else {
639+
processedVal = nil
640+
}
627641
case time.Time:
628642
processedVal = v
629643
case string:

internal/consistency/repair/table_repair.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/google/uuid"
2828
"github.com/jackc/pglogrepl"
2929
"github.com/jackc/pgx/v5"
30+
pgxv5type "github.com/jackc/pgx/v5/pgtype"
3031
"github.com/jackc/pgx/v5/pgxpool"
3132
"github.com/pgedge/ace/db/queries"
3233
planner "github.com/pgedge/ace/internal/consistency/repair/plan"
@@ -1227,6 +1228,13 @@ func convertValueForType(val any, colType string) (any, error) {
12271228
}
12281229

12291230
if tVal, ok := val.(time.Time); ok {
1231+
// For "timestamp without time zone" columns, wrap in pgxv5type.Timestamp
1232+
// so pgx sends the value as "timestamp" instead of "timestamptz". This prevents
1233+
// PostgreSQL from applying a session-timezone conversion.
1234+
lower := strings.ToLower(colType)
1235+
if lower == "timestamp without time zone" || (strings.HasPrefix(lower, "timestamp") && !strings.Contains(lower, "with time zone")) {
1236+
return pgxv5type.Timestamp{Time: tVal, Valid: true}, nil
1237+
}
12301238
return tVal, nil
12311239
}
12321240

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
// ///////////////////////////////////////////////////////////////////////////
2+
//
3+
// # ACE - Active Consistency Engine
4+
//
5+
// Copyright (C) 2023 - 2026, pgEdge (https://www.pgedge.com/)
6+
//
7+
// This software is released under the PostgreSQL License:
8+
// https://opensource.org/license/postgresql
9+
//
10+
// ///////////////////////////////////////////////////////////////////////////
11+
12+
package repair
13+
14+
import (
15+
"strings"
16+
"testing"
17+
"time"
18+
19+
pgxv5type "github.com/jackc/pgx/v5/pgtype"
20+
"github.com/stretchr/testify/require"
21+
)
22+
23+
func TestConvertValueForType_TimestampWrapsInPgxTimestamp(t *testing.T) {
24+
// When a time.Time value is passed for a "timestamp without time zone"
25+
// column, convertValueForType must wrap it in pgxv5type.Timestamp so
26+
// pgx sends the value as "timestamp" instead of "timestamptz". This prevents PostgreSQL from
27+
// applying a session-timezone conversion that shifts the value.
28+
ts := time.Date(2024, 6, 15, 10, 30, 0, 0, time.UTC)
29+
30+
tests := []struct {
31+
name string
32+
colType string
33+
wantPgx bool // true → expect pgxv5type.Timestamp; false → expect time.Time
34+
}{
35+
{"timestamp without time zone", "timestamp without time zone", true},
36+
{"TIMESTAMP WITHOUT TIME ZONE (upper)", "TIMESTAMP WITHOUT TIME ZONE", true},
37+
{"timestamp with time zone", "timestamp with time zone", false},
38+
{"timestamptz via format_type", "timestamp with time zone", false},
39+
}
40+
41+
for _, tt := range tests {
42+
t.Run(tt.name, func(t *testing.T) {
43+
got, err := convertValueForType(ts, tt.colType)
44+
require.NoError(t, err)
45+
46+
if tt.wantPgx {
47+
pgTS, ok := got.(pgxv5type.Timestamp)
48+
require.True(t, ok, "expected pgxv5type.Timestamp, got %T", got)
49+
require.True(t, pgTS.Valid)
50+
require.True(t, pgTS.Time.Equal(ts))
51+
} else {
52+
tv, ok := got.(time.Time)
53+
require.True(t, ok, "expected time.Time, got %T", got)
54+
require.True(t, tv.Equal(ts))
55+
}
56+
})
57+
}
58+
}
59+
60+
func TestConvertValueForType_StringTimestamp(t *testing.T) {
61+
// Values coming from diff JSON are strings. They should be parsed and
62+
// wrapped in pgxv5type.Timestamp for "timestamp without time zone".
63+
val, err := convertValueForType("2024-06-15T10:30:00Z", "timestamp without time zone")
64+
require.NoError(t, err)
65+
66+
pgTS, ok := val.(pgxv5type.Timestamp)
67+
require.True(t, ok, "expected pgxv5type.Timestamp, got %T", val)
68+
require.True(t, pgTS.Valid)
69+
70+
expected := time.Date(2024, 6, 15, 10, 30, 0, 0, time.UTC)
71+
require.True(t, pgTS.Time.Equal(expected))
72+
}
73+
74+
func TestConvertValueForType_StringTimestamptz(t *testing.T) {
75+
// Values for "timestamp with time zone" columns should remain time.Time.
76+
val, err := convertValueForType("2024-06-15T10:30:00Z", "timestamp with time zone")
77+
require.NoError(t, err)
78+
79+
tv, ok := val.(time.Time)
80+
require.True(t, ok, "expected time.Time, got %T", val)
81+
82+
expected := time.Date(2024, 6, 15, 10, 30, 0, 0, time.UTC)
83+
require.True(t, tv.Equal(expected))
84+
}
85+
86+
func TestConvertValueForType_Nil(t *testing.T) {
87+
val, err := convertValueForType(nil, "timestamp without time zone")
88+
require.NoError(t, err)
89+
require.Nil(t, val)
90+
}
91+
92+
func TestBuildFixNullsBatchSQL_TimestampType(t *testing.T) {
93+
// Verify that buildFixNullsBatchSQL generates valid SQL for timestamp
94+
// columns and that values are properly converted.
95+
task := &TableRepairTask{}
96+
task.Schema = "public"
97+
task.Table = "test_table"
98+
task.Key = []string{"id"}
99+
100+
colTypes := map[string]string{
101+
"id": "integer",
102+
"ts_col": "timestamp without time zone",
103+
}
104+
105+
batch := []*nullUpdate{
106+
{
107+
pkValues: []any{float64(1)},
108+
columns: map[string]any{"ts_col": "2024-06-15T10:30:00Z"},
109+
},
110+
}
111+
112+
sql, args, err := task.buildFixNullsBatchSQL("ts_col", "timestamp without time zone", batch, colTypes)
113+
require.NoError(t, err)
114+
115+
// The SQL should contain the correct type cast
116+
require.True(t, strings.Contains(sql, "::timestamp without time zone"),
117+
"expected SQL to contain '::timestamp without time zone', got: %s", sql)
118+
119+
// The args should contain the converted values
120+
require.Len(t, args, 2) // 1 PK + 1 value
121+
122+
// The timestamp value should be pgxv5type.Timestamp, not time.Time
123+
pgTS, ok := args[1].(pgxv5type.Timestamp)
124+
require.True(t, ok, "expected timestamp arg to be pgxv5type.Timestamp, got %T", args[1])
125+
require.True(t, pgTS.Valid)
126+
127+
expected := time.Date(2024, 6, 15, 10, 30, 0, 0, time.UTC)
128+
require.True(t, pgTS.Time.Equal(expected))
129+
}
130+
131+
func TestBuildFixNullsBatchSQL_TimestamptzType(t *testing.T) {
132+
// Verify that timestamptz columns use time.Time (not pgxv5type.Timestamp).
133+
task := &TableRepairTask{}
134+
task.Schema = "public"
135+
task.Table = "test_table"
136+
task.Key = []string{"id"}
137+
138+
colTypes := map[string]string{
139+
"id": "integer",
140+
"tstz_col": "timestamp with time zone",
141+
}
142+
143+
batch := []*nullUpdate{
144+
{
145+
pkValues: []any{float64(1)},
146+
columns: map[string]any{"tstz_col": "2024-06-15T10:30:00Z"},
147+
},
148+
}
149+
150+
sql, args, err := task.buildFixNullsBatchSQL("tstz_col", "timestamp with time zone", batch, colTypes)
151+
require.NoError(t, err)
152+
153+
require.True(t, strings.Contains(sql, "::timestamp with time zone"),
154+
"expected SQL to contain '::timestamp with time zone', got: %s", sql)
155+
156+
require.Len(t, args, 2)
157+
158+
// The timestamptz value should be a plain time.Time
159+
tv, ok := args[1].(time.Time)
160+
require.True(t, ok, "expected timestamptz arg to be time.Time, got %T", args[1])
161+
162+
expected := time.Date(2024, 6, 15, 10, 30, 0, 0, time.UTC)
163+
require.True(t, tv.Equal(expected))
164+
}
165+
166+
func TestConvertValueForType_StringTime(t *testing.T) {
167+
// Values from diff JSON for "time without time zone" columns should be
168+
// parsed into pgxv5type.Time via ConvertToPgxType.
169+
val, err := convertValueForType("10:30:00.000000", "time without time zone")
170+
require.NoError(t, err)
171+
172+
tv, ok := val.(pgxv5type.Time)
173+
require.True(t, ok, "expected pgxv5type.Time, got %T", val)
174+
require.True(t, tv.Valid)
175+
176+
expectedUsec := int64(10*3_600_000_000 + 30*60_000_000)
177+
require.Equal(t, expectedUsec, tv.Microseconds)
178+
}
179+
180+
func TestConvertValueForType_StringTimetz(t *testing.T) {
181+
// Values for "time with time zone" columns should remain as strings.
182+
val, err := convertValueForType("10:30:00-05:00", "time with time zone")
183+
require.NoError(t, err)
184+
185+
s, ok := val.(string)
186+
require.True(t, ok, "expected string, got %T", val)
187+
require.Equal(t, "10:30:00-05:00", s)
188+
}
189+
190+
func TestBuildFixNullsBatchSQL_TimeType(t *testing.T) {
191+
// Verify that buildFixNullsBatchSQL generates valid SQL for time columns
192+
// and that values are properly converted to pgxv5type.Time.
193+
task := &TableRepairTask{}
194+
task.Schema = "public"
195+
task.Table = "test_table"
196+
task.Key = []string{"id"}
197+
198+
colTypes := map[string]string{
199+
"id": "integer",
200+
"time_col": "time without time zone",
201+
}
202+
203+
batch := []*nullUpdate{
204+
{
205+
pkValues: []any{float64(1)},
206+
columns: map[string]any{"time_col": "14:30:00.000000"},
207+
},
208+
}
209+
210+
sql, args, err := task.buildFixNullsBatchSQL("time_col", "time without time zone", batch, colTypes)
211+
require.NoError(t, err)
212+
213+
require.True(t, strings.Contains(sql, "::time without time zone"),
214+
"expected SQL to contain '::time without time zone', got: %s", sql)
215+
216+
require.Len(t, args, 2) // 1 PK + 1 value
217+
218+
tv, ok := args[1].(pgxv5type.Time)
219+
require.True(t, ok, "expected time arg to be pgxv5type.Time, got %T", args[1])
220+
require.True(t, tv.Valid)
221+
222+
expectedUsec := int64(14*3_600_000_000 + 30*60_000_000)
223+
require.Equal(t, expectedUsec, tv.Microseconds)
224+
}
225+
226+
func TestBuildFixNullsBatchSQL_TimetzType(t *testing.T) {
227+
// Verify that timetz columns pass through as strings.
228+
task := &TableRepairTask{}
229+
task.Schema = "public"
230+
task.Table = "test_table"
231+
task.Key = []string{"id"}
232+
233+
colTypes := map[string]string{
234+
"id": "integer",
235+
"timetz_col": "time with time zone",
236+
}
237+
238+
batch := []*nullUpdate{
239+
{
240+
pkValues: []any{float64(1)},
241+
columns: map[string]any{"timetz_col": "14:30:00-05:00"},
242+
},
243+
}
244+
245+
sql, args, err := task.buildFixNullsBatchSQL("timetz_col", "time with time zone", batch, colTypes)
246+
require.NoError(t, err)
247+
248+
require.True(t, strings.Contains(sql, "::time with time zone"),
249+
"expected SQL to contain '::time with time zone', got: %s", sql)
250+
251+
require.Len(t, args, 2)
252+
253+
s, ok := args[1].(string)
254+
require.True(t, ok, "expected timetz arg to be string, got %T", args[1])
255+
require.Equal(t, "14:30:00-05:00", s)
256+
}

0 commit comments

Comments
 (0)