Skip to content

Commit 08c9bb1

Browse files
jackcclaude
andcommitted
Fix Stringer types encoded as text instead of numeric value in composite fields
Named integer types implementing fmt.Stringer (e.g. protobuf enums) were encoded using their String() output instead of their numeric value when used inside composite type fields. This happened because TryWrapBuiltinTypeEncodePlan matched fmt.Stringer before TryWrapFindUnderlyingTypeEncodePlan could convert the type to its underlying integer. Extract fmt.Stringer handling into a new TryWrapStringerEncodePlan function that runs after TryWrapFindUnderlyingTypeEncodePlan. The wrapper loop's existing "first successful recursive plan wins" semantics ensures the right behavior: numeric codecs match for numeric OIDs, Stringer is used as fallback for text OIDs. Fixes #2527 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 96b4dbd commit 08c9bb1

2 files changed

Lines changed: 80 additions & 2 deletions

File tree

pgtype/composite_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,67 @@ create type ct_test as (
4949
})
5050
}
5151

52+
// stringerInt32 is a custom int32 type that implements fmt.Stringer, similar to protobuf enums.
53+
type stringerInt32 int32
54+
55+
const (
56+
stringerInt32Foo stringerInt32 = 100
57+
stringerInt32Bar stringerInt32 = 200
58+
)
59+
60+
func (s stringerInt32) String() string {
61+
switch s {
62+
case stringerInt32Foo:
63+
return "FOO"
64+
case stringerInt32Bar:
65+
return "BAR"
66+
default:
67+
return "UNKNOWN"
68+
}
69+
}
70+
71+
// https://github.com/jackc/pgx/discussions/2527
72+
func TestCompositeCodecTranscodeWithStringerInt(t *testing.T) {
73+
defaultConnTestRunner.RunTest(context.Background(), t, func(ctx context.Context, t testing.TB, conn *pgx.Conn) {
74+
_, err := conn.Exec(ctx, `drop type if exists ct_stringer_test;
75+
76+
create type ct_stringer_test as (
77+
a text,
78+
b smallint
79+
);`)
80+
require.NoError(t, err)
81+
defer conn.Exec(ctx, "drop type ct_stringer_test")
82+
83+
dt, err := conn.LoadType(ctx, "ct_stringer_test")
84+
require.NoError(t, err)
85+
conn.TypeMap().RegisterType(dt)
86+
87+
formats := []struct {
88+
name string
89+
code int16
90+
}{
91+
{name: "TextFormat", code: pgx.TextFormatCode},
92+
{name: "BinaryFormat", code: pgx.BinaryFormatCode},
93+
}
94+
95+
// Pass the same stringerInt32 value for both fields. The text field should get the Stringer value
96+
// ("BAR") and the smallint field should get the integer value (200).
97+
for _, format := range formats {
98+
var a string
99+
var b int16
100+
101+
err := conn.QueryRow(ctx, "select $1::ct_stringer_test", pgx.QueryResultFormats{format.code},
102+
pgtype.CompositeFields{stringerInt32Bar, stringerInt32Bar},
103+
).Scan(
104+
pgtype.CompositeFields{&a, &b},
105+
)
106+
require.NoErrorf(t, err, "%v", format.name)
107+
require.EqualValuesf(t, "BAR", a, "%v", format.name)
108+
require.EqualValuesf(t, 200, b, "%v", format.name)
109+
}
110+
})
111+
}
112+
52113
type point3d struct {
53114
X, Y, Z float64
54115
}

pgtype/pgtype.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ func NewMap() *Map {
243243
TryWrapDerefPointerEncodePlan,
244244
TryWrapBuiltinTypeEncodePlan,
245245
TryWrapFindUnderlyingTypeEncodePlan,
246+
TryWrapStringerEncodePlan,
246247
TryWrapStructEncodePlan,
247248
TryWrapSliceEncodePlan,
248249
TryWrapMultiDimSliceEncodePlan,
@@ -1446,6 +1447,24 @@ func TryWrapFindUnderlyingTypeEncodePlan(value any) (plan WrappedEncodePlanNextS
14461447
return nil, nil, false
14471448
}
14481449

1450+
// TryWrapStringerEncodePlan tries to wrap a fmt.Stringer type with a wrapper that provides TextValuer. This is
1451+
// intentionally a separate function from TryWrapBuiltinTypeEncodePlan so it can be ordered after
1452+
// TryWrapFindUnderlyingTypeEncodePlan. This ensures that named types with an underlying builtin type (e.g. type MyEnum
1453+
// int32 with a String() method) prefer encoding via the underlying type's codec (e.g. as an integer) rather than via
1454+
// Stringer. Stringer is only used as a fallback when no type-specific encoding plan succeeds.
1455+
// (https://github.com/jackc/pgx/discussions/2527)
1456+
func TryWrapStringerEncodePlan(value any) (plan WrappedEncodePlanNextSetter, nextValue any, ok bool) {
1457+
if _, ok := value.(driver.Valuer); ok {
1458+
return nil, nil, false
1459+
}
1460+
1461+
if s, ok := value.(fmt.Stringer); ok {
1462+
return &wrapFmtStringerEncodePlan{}, fmtStringerWrapper{s}, true
1463+
}
1464+
1465+
return nil, nil, false
1466+
}
1467+
14491468
type WrappedEncodePlanNextSetter interface {
14501469
SetNext(EncodePlan)
14511470
EncodePlan
@@ -1506,8 +1525,6 @@ func TryWrapBuiltinTypeEncodePlan(value any) (plan WrappedEncodePlanNextSetter,
15061525
return &wrapByte16EncodePlan{}, byte16Wrapper(value), true
15071526
case []byte:
15081527
return &wrapByteSliceEncodePlan{}, byteSliceWrapper(value), true
1509-
case fmt.Stringer:
1510-
return &wrapFmtStringerEncodePlan{}, fmtStringerWrapper{value}, true
15111528
}
15121529

15131530
return nil, nil, false

0 commit comments

Comments
 (0)