Skip to content

Commit 13beb38

Browse files
jackcjackc
committed
Fix encode driver.Valuer on nil-able non-pointers
#1566 #1860 #2019 (comment)
1 parent fec45c8 commit 13beb38

File tree

4 files changed

+130
-20
lines changed

4 files changed

+130
-20
lines changed

‎extended_query_builder.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,11 @@ func (eqb *ExtendedQueryBuilder) oidAndArgForQueryExecModeExec(m *pgtype.Map, ar
198198
if err != nil {
199199
return 0, nil, err
200200
}
201+
202+
if v == nil {
203+
return 0, nil, nil
204+
}
205+
201206
if dt, ok := m.TypeForValue(v); ok {
202207
return dt.OID, v, nil
203208
}

‎internal/anynil/anynil.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ import (
1414
// var valuerReflectType = reflect.TypeFor[driver.Valuer]()
1515
var valuerReflectType = reflect.TypeOf((*driver.Valuer)(nil)).Elem()
1616

17-
// Is returns true if value is any type of nil except a pointer that directly implements driver.Valuer. e.g. nil,
18-
// []byte(nil), and a *T where T implements driver.Valuer get normalized to nil but a *T where *T implements
19-
// driver.Valuer does not.
17+
// Is returns true if value is any type of nil unless it implements driver.Valuer. *T is not considered to implement
18+
// driver.Valuer if it is only implemented by T.
2019
func Is(value any) bool {
2120
if value == nil {
2221
return true
@@ -30,14 +29,13 @@ func Is(value any) bool {
3029
return false
3130
}
3231

33-
if kind == reflect.Ptr {
34-
if _, ok := value.(driver.Valuer); ok {
35-
// The pointer will be considered to implement driver.Valuer even if it is actually implemented on the value.
36-
// But we only want to consider it nil if it is implemented on the pointer. So check if what the pointer points
37-
// to implements driver.Valuer.
38-
if !refVal.Type().Elem().Implements(valuerReflectType) {
39-
return false
40-
}
32+
if _, ok := value.(driver.Valuer); ok {
33+
if kind == reflect.Ptr {
34+
// The type assertion will succeed if driver.Valuer is implemented on T or *T. Check if it is implemented on T
35+
// to see if it is not implemented on *T.
36+
return refVal.Type().Elem().Implements(valuerReflectType)
37+
} else {
38+
return false
4139
}
4240
}
4341

‎pgtype/doc.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,10 @@ Encoding Typed Nils
144144
pgtype normalizes typed nils (e.g. []byte(nil)) into nil. nil is always encoded is the SQL NULL value without going
145145
through the Codec system. This means that Codecs and other encoding logic does not have to handle nil or *T(nil).
146146
147-
However, database/sql compatibility requires Value to be called on a pointer that implements driver.Valuer. See
147+
However, database/sql compatibility requires Value to be called on T(nil) when T implements driver.Valuer. Therefore,
148+
driver.Valuer values are not normalized to nil unless it is a *T(nil) where driver.Valuer is implemented on T. See
148149
https://github.com/golang/go/issues/8415 and
149-
https://github.com/golang/go/commit/0ce1d79a6a771f7449ec493b993ed2a720917870. Therefore, pointers that implement
150-
driver.Valuer are not normalized to nil.
150+
https://github.com/golang/go/commit/0ce1d79a6a771f7449ec493b993ed2a720917870.
151151
152152
Child Records
153153

‎query_test.go

Lines changed: 113 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,12 +1173,12 @@ func TestConnQueryDatabaseSQLDriverValuerWithAutoGeneratedPointerReceiver(t *tes
11731173
ensureConnValid(t, conn)
11741174
}
11751175

1176-
type nilAsEmptyJSONObject struct {
1176+
type nilPointerAsEmptyJSONObject struct {
11771177
ID string
11781178
Name string
11791179
}
11801180

1181-
func (v *nilAsEmptyJSONObject) Value() (driver.Value, error) {
1181+
func (v *nilPointerAsEmptyJSONObject) Value() (driver.Value, error) {
11821182
if v == nil {
11831183
return "{}", nil
11841184
}
@@ -1187,15 +1187,15 @@ func (v *nilAsEmptyJSONObject) Value() (driver.Value, error) {
11871187
}
11881188

11891189
// https://github.com/jackc/pgx/issues/1566
1190-
func TestConnQueryDatabaseSQLDriverValuerCalledOnPointerImplementers(t *testing.T) {
1190+
func TestConnQueryDatabaseSQLDriverValuerCalledOnNilPointerImplementers(t *testing.T) {
11911191
t.Parallel()
11921192

11931193
conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE"))
11941194
defer closeConn(t, conn)
11951195

11961196
mustExec(t, conn, "create temporary table t(v json not null)")
11971197

1198-
var v *nilAsEmptyJSONObject
1198+
var v *nilPointerAsEmptyJSONObject
11991199
commandTag, err := conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
12001200
require.NoError(t, err)
12011201
require.Equal(t, "INSERT 0 1", commandTag.String())
@@ -1208,12 +1208,119 @@ func TestConnQueryDatabaseSQLDriverValuerCalledOnPointerImplementers(t *testing.
12081208
_, err = conn.Exec(context.Background(), `delete from t`)
12091209
require.NoError(t, err)
12101210

1211-
v = &nilAsEmptyJSONObject{ID: "1", Name: "foo"}
1211+
v = &nilPointerAsEmptyJSONObject{ID: "1", Name: "foo"}
12121212
commandTag, err = conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
12131213
require.NoError(t, err)
12141214
require.Equal(t, "INSERT 0 1", commandTag.String())
12151215

1216-
var v2 *nilAsEmptyJSONObject
1216+
var v2 *nilPointerAsEmptyJSONObject
1217+
err = conn.QueryRow(context.Background(), "select v from t").Scan(&v2)
1218+
require.NoError(t, err)
1219+
require.Equal(t, v, v2)
1220+
1221+
ensureConnValid(t, conn)
1222+
}
1223+
1224+
type nilSliceAsEmptySlice []byte
1225+
1226+
func (j nilSliceAsEmptySlice) Value() (driver.Value, error) {
1227+
if len(j) == 0 {
1228+
return []byte("[]"), nil
1229+
}
1230+
1231+
return []byte(j), nil
1232+
}
1233+
1234+
func (j *nilSliceAsEmptySlice) UnmarshalJSON(data []byte) error {
1235+
*j = bytes.Clone(data)
1236+
return nil
1237+
}
1238+
1239+
// https://github.com/jackc/pgx/issues/1860
1240+
func TestConnQueryDatabaseSQLDriverValuerCalledOnNilSliceImplementers(t *testing.T) {
1241+
t.Parallel()
1242+
1243+
conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE"))
1244+
defer closeConn(t, conn)
1245+
1246+
mustExec(t, conn, "create temporary table t(v json not null)")
1247+
1248+
var v nilSliceAsEmptySlice
1249+
commandTag, err := conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
1250+
require.NoError(t, err)
1251+
require.Equal(t, "INSERT 0 1", commandTag.String())
1252+
1253+
var s string
1254+
err = conn.QueryRow(context.Background(), "select v from t").Scan(&s)
1255+
require.NoError(t, err)
1256+
require.Equal(t, "[]", s)
1257+
1258+
_, err = conn.Exec(context.Background(), `delete from t`)
1259+
require.NoError(t, err)
1260+
1261+
v = nilSliceAsEmptySlice(`{"name": "foo"}`)
1262+
commandTag, err = conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
1263+
require.NoError(t, err)
1264+
require.Equal(t, "INSERT 0 1", commandTag.String())
1265+
1266+
var v2 nilSliceAsEmptySlice
1267+
err = conn.QueryRow(context.Background(), "select v from t").Scan(&v2)
1268+
require.NoError(t, err)
1269+
require.Equal(t, v, v2)
1270+
1271+
ensureConnValid(t, conn)
1272+
}
1273+
1274+
type nilMapAsEmptyObject map[string]any
1275+
1276+
func (j nilMapAsEmptyObject) Value() (driver.Value, error) {
1277+
if j == nil {
1278+
return []byte("{}"), nil
1279+
}
1280+
1281+
return json.Marshal(j)
1282+
}
1283+
1284+
func (j *nilMapAsEmptyObject) UnmarshalJSON(data []byte) error {
1285+
var m map[string]any
1286+
err := json.Unmarshal(data, &m)
1287+
if err != nil {
1288+
return err
1289+
}
1290+
1291+
*j = m
1292+
1293+
return nil
1294+
}
1295+
1296+
// https://github.com/jackc/pgx/pull/2019#discussion_r1605806751
1297+
func TestConnQueryDatabaseSQLDriverValuerCalledOnNilMapImplementers(t *testing.T) {
1298+
t.Parallel()
1299+
1300+
conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE"))
1301+
defer closeConn(t, conn)
1302+
1303+
mustExec(t, conn, "create temporary table t(v json not null)")
1304+
1305+
var v nilMapAsEmptyObject
1306+
commandTag, err := conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
1307+
require.NoError(t, err)
1308+
require.Equal(t, "INSERT 0 1", commandTag.String())
1309+
1310+
var s string
1311+
err = conn.QueryRow(context.Background(), "select v from t").Scan(&s)
1312+
require.NoError(t, err)
1313+
require.Equal(t, "{}", s)
1314+
1315+
_, err = conn.Exec(context.Background(), `delete from t`)
1316+
require.NoError(t, err)
1317+
1318+
v = nilMapAsEmptyObject{"name": "foo"}
1319+
commandTag, err = conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
1320+
require.NoError(t, err)
1321+
require.Equal(t, "INSERT 0 1", commandTag.String())
1322+
1323+
var v2 nilMapAsEmptyObject
12171324
err = conn.QueryRow(context.Background(), "select v from t").Scan(&v2)
12181325
require.NoError(t, err)
12191326
require.Equal(t, v, v2)

0 commit comments

Comments
 (0)