Skip to content

Commit a82e4ff

Browse files
committed
fix(diff): cast daterange and rerun row fetches to text so repairs work
IsKnownScalarType matched format_type names by prefix, so "daterange" passed as "date" and was the one complex type still fetched un-cast -- its Go-struct form in the diff report failed the whole upsert batch on repair (malformed range literal). Strip the typmod and require an exact match, which also covers datemultirange and UDTs sharing a scalar's prefix. table-rerun's row fetch never applied the cast policy at all; it now consults the same NeedsTextCast so rerun reports stay repairable. The typed-columns integration test gains a daterange column and a rerun leg; the cast-policy decision table covers the collision cases.
1 parent beecb35 commit a82e4ff

4 files changed

Lines changed: 75 additions & 21 deletions

File tree

internal/consistency/diff/table_rerun.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,22 @@ func fetchRowsByPkeys(ctx context.Context, pool *pgxpool.Pool, t *TableDiffTask,
276276
joinConditions = append(joinConditions, fmt.Sprintf("t.%s = temp.%s", sanitisedPkCol, sanitisedPkCol))
277277
}
278278

279+
colTypes, err := queries.GetColumnTypes(ctx, tx, t.Schema, t.Table)
280+
if err != nil {
281+
return nil, fmt.Errorf("could not determine column types: %w", err)
282+
}
283+
279284
selectCols := make([]string, 0, len(t.Cols)+2)
280285
selectCols = append(selectCols, "pg_xact_commit_timestamp(t.xmin) as commit_ts", "to_json(pg_xact_commit_timestamp_origin(t.xmin))->>'roident' as node_origin")
281286
for _, col := range t.Cols {
282-
selectCols = append(selectCols, "t."+pgx.Identifier{col}.Sanitize())
287+
quotedCol := pgx.Identifier{col}.Sanitize()
288+
// Same cast policy as the diff engines' row fetches: complex types
289+
// must travel as Postgres text so the rerun report stays repairable.
290+
if utils.NeedsTextCast(colTypes[col]) {
291+
selectCols = append(selectCols, fmt.Sprintf("t.%s::TEXT AS %s", quotedCol, quotedCol))
292+
} else {
293+
selectCols = append(selectCols, "t."+quotedCol)
294+
}
283295
}
284296

285297
fetchSQL := fmt.Sprintf("SELECT %s FROM %s t JOIN %s temp ON %s",

pkg/common/utils.go

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,37 +1003,52 @@ func SafeCut(s string, n int) string {
10031003
// conversion code. Both diff engines must build their row-fetch SELECTs with
10041004
// it so reports stay engine-independent and repairable.
10051005
func SelectColExpr(quotedCol, colType string) string {
1006-
if strings.HasSuffix(colType, "[]") ||
1007-
strings.Contains(strings.ToLower(colType), "json") ||
1008-
strings.Contains(strings.ToLower(colType), "bytea") ||
1009-
!IsKnownScalarType(colType) {
1006+
if NeedsTextCast(colType) {
10101007
return fmt.Sprintf("%s::TEXT AS %s", quotedCol, quotedCol)
10111008
}
10121009
return quotedCol
10131010
}
10141011

1012+
// NeedsTextCast reports whether a diff row fetch must cast the column to
1013+
// ::TEXT for its value to round-trip through a diff report (see
1014+
// SelectColExpr). Row-fetch sites that cannot use SelectColExpr verbatim
1015+
// (e.g. table-qualified selects) must still consult this policy.
1016+
func NeedsTextCast(colType string) bool {
1017+
return strings.HasSuffix(colType, "[]") ||
1018+
strings.Contains(strings.ToLower(colType), "json") ||
1019+
strings.Contains(strings.ToLower(colType), "bytea") ||
1020+
!IsKnownScalarType(colType)
1021+
}
1022+
10151023
func IsKnownScalarType(colType string) bool {
1016-
// "time with time zone" (timetz) is NOT registered in the pgx v5
1017-
// default type map, so it cannot be scanned into *interface{}. Exclude it
1018-
// so the diff layer casts it to ::TEXT.
1019-
if strings.HasPrefix(colType, "time with time zone") {
1020-
return false
1024+
// colType is pg_catalog.format_type output. Strip the typmod so
1025+
// "numeric(10,2)" and "timestamp(3) with time zone" match their base
1026+
// names, then require an exact match: prefix matching mistook types that
1027+
// merely start like a scalar ("daterange" for "date") for the scalar
1028+
// itself, so they were never cast to ::TEXT and reached diff reports as
1029+
// unrepairable Go-formatted structs.
1030+
base := colType
1031+
if i := strings.IndexByte(base, '('); i >= 0 {
1032+
if j := strings.IndexByte(base[i:], ')'); j >= 0 {
1033+
base = strings.TrimSpace(base[:i] + base[i+j+1:])
1034+
}
10211035
}
10221036

1023-
knownPrefixes := []string{
1024-
"character", "text",
1037+
switch base {
1038+
case "character varying", "character", "text",
10251039
"integer", "bigint", "smallint",
10261040
"numeric", "decimal", "real", "double precision",
10271041
"boolean",
10281042
"bytea",
10291043
"json", "jsonb",
10301044
"uuid",
1031-
"timestamp", "date", "time",
1032-
}
1033-
for _, prefix := range knownPrefixes {
1034-
if strings.HasPrefix(colType, prefix) {
1035-
return true
1036-
}
1045+
"date",
1046+
"timestamp without time zone", "timestamp with time zone",
1047+
// "time with time zone" (timetz) is deliberately absent: it is not
1048+
// registered in the pgx v5 default type map, so it cannot be scanned
1049+
// into *interface{} and must be cast to ::TEXT by the diff layer.
1050+
"time without time zone":
1051+
return true
10371052
}
10381053
return false
10391054
}

pkg/common/utils_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,20 @@ func TestSelectColExpr(t *testing.T) {
132132
}{
133133
{"integer", `"c"`},
134134
{"timestamp without time zone", `"c"`},
135+
{"timestamp(3) with time zone", `"c"`},
136+
{"numeric(10,2)", `"c"`},
137+
{"date", `"c"`},
135138
{"uuid", `"c"`},
136139
{"integer[]", `"c"::TEXT AS "c"`},
137140
{"jsonb", `"c"::TEXT AS "c"`},
138141
{"bytea", `"c"::TEXT AS "c"`},
139142
{"point", `"c"::TEXT AS "c"`},
140143
{"int4range", `"c"::TEXT AS "c"`},
144+
// "daterange" must not be mistaken for "date" (nor "datemultirange",
145+
// nor a UDT that happens to share a scalar's name as a prefix).
146+
{"daterange", `"c"::TEXT AS "c"`},
147+
{"datemultirange", `"c"::TEXT AS "c"`},
148+
{"date_status_enum", `"c"::TEXT AS "c"`},
141149
{"mood_enum", `"c"::TEXT AS "c"`},
142150
{"xml", `"c"::TEXT AS "c"`},
143151
{"bit(8)", `"c"::TEXT AS "c"`},

tests/integration/mtree_typed_repair_test.go

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,20 @@ import (
2121
"github.com/jackc/pgx/v5"
2222
"github.com/jackc/pgx/v5/pgxpool"
2323
"github.com/stretchr/testify/require"
24+
25+
"github.com/pgedge/ace/internal/consistency/diff"
2426
)
2527

2628
// typedRepairDDL covers the type classes repair must round-trip: temporal,
2729
// money, uuid, interval, numeric, plus the complex ones the driver scans as
2830
// opaque structs (geometric, range, bit, network, enum, xml).
2931
const typedRepairDDL = ` (id BIGINT PRIMARY KEY, name VARCHAR(100), col_time TIME, col_timetz TIMETZ,
3032
col_money MONEY, col_uuid UUID, col_interval INTERVAL, col_num NUMERIC(12,4),
31-
col_point POINT, col_range INT4RANGE, col_bit BIT(8), col_inet INET,
32-
col_xml XML, col_mood typed_repair_mood)`
33+
col_point POINT, col_range INT4RANGE, col_daterange DATERANGE, col_bit BIT(8),
34+
col_inet INET, col_xml XML, col_mood typed_repair_mood)`
3335

3436
const typedRepairSeedSQL = ` (id, name, col_time, col_timetz, col_money, col_uuid, col_interval, col_num,
35-
col_point, col_range, col_bit, col_inet, col_xml, col_mood)
37+
col_point, col_range, col_daterange, col_bit, col_inet, col_xml, col_mood)
3638
SELECT i,
3739
'name_' || i,
3840
TIME '00:00:00' + (i * 137 || ' seconds')::interval,
@@ -43,6 +45,7 @@ const typedRepairSeedSQL = ` (id, name, col_time, col_timetz, col_money, col_uui
4345
i * 1.5,
4446
point(i, i * 2),
4547
int4range(i, i + 100),
48+
daterange(DATE '2020-01-01' + i, DATE '2020-01-01' + i + 30),
4649
(i % 256)::bit(8),
4750
('10.0.' || (i % 256) || '.' || (i % 200 + 1))::inet,
4851
('<item id="' || i || '"><name>item_' || i || '</name></item>')::xml,
@@ -148,4 +151,20 @@ func TestMtreeDiffRepairTypedColumns(t *testing.T) {
148151

149152
require.Equal(t, typedRepairFingerprint(t, ctx, env.N1Pool, safe), typedRepairFingerprint(t, ctx, env.N2Pool, safe),
150153
"nodes must hold identical rows (all typed columns included) after repair")
154+
155+
// table-rerun re-fetches the diffed rows by primary key -- a separate
156+
// SELECT that must apply the same ::TEXT cast policy, or typed columns
157+
// fail the scan (timetz) or re-compare as spurious diffs. On the now
158+
// converged nodes it must succeed and confirm every diff resolved.
159+
rerunTask := diff.NewTableDiffTask()
160+
rerunTask.Mode = "rerun"
161+
rerunTask.ClusterName = env.ClusterName
162+
rerunTask.DBName = env.DBName
163+
rerunTask.DiffFilePath = diffFile
164+
rerunTask.Ctx = ctx
165+
require.NoError(t, rerunTask.ExecuteTask(), "table-rerun must succeed on typed columns")
166+
167+
rerunReports, err := filepath.Glob(fmt.Sprintf("%s_%s_rerun-diffs-*.json", testSchema, tableName))
168+
require.NoError(t, err)
169+
require.Empty(t, rerunReports, "rerun on converged nodes must not report persistent diffs")
151170
}

0 commit comments

Comments
 (0)