Fix encode driver.Valuer on pointer

README.md

@@ -92,7 +92,7 @@ See the presentation at Golang Estonia, [PGX Top to Bottom](https://www.youtube.
 
 ## Supported Go and PostgreSQL Versions
 
-pgx supports the same versions of Go and PostgreSQL that are supported by their respective teams. For [Go](https://golang.org/doc/devel/release.html#policy) that is the two most recent major releases and for [PostgreSQL](https://www.postgresql.org/support/versioning/) the major releases in the last 5 years. This means pgx supports Go 1.20 and higher and PostgreSQL 12 and higher. pgx also is tested against the latest version of [CockroachDB](https://www.cockroachlabs.com/product/).
+pgx supports the same versions of Go and PostgreSQL that are supported by their respective teams. For [Go](https://golang.org/doc/devel/release.html#policy) that is the two most recent major releases and for [PostgreSQL](https://www.postgresql.org/support/versioning/) the major releases in the last 5 years. This means pgx supports Go 1.21 and higher and PostgreSQL 12 and higher. pgx also is tested against the latest version of [CockroachDB](https://www.cockroachlabs.com/product/).
 
 ## Version Policy
 

extended_query_builder.go

@@ -161,58 +161,56 @@ func (eqb *ExtendedQueryBuilder) chooseParameterFormatCode(m *pgtype.Map, oid ui
 // no way to safely use binary or to specify the parameter OIDs.
 func (eqb *ExtendedQueryBuilder) appendParamsForQueryExecModeExec(m *pgtype.Map, args []any) error {
 	for _, arg := range args {
-		if arg == nil {
-			err := eqb.appendParam(m, 0, TextFormatCode, arg)
-			if err != nil {
-				return err
-			}
-		} else {
-			dt, ok := m.TypeForValue(arg)
-			if !ok {
-				var tv pgtype.TextValuer
-				if tv, ok = arg.(pgtype.TextValuer); ok {
-					t, err := tv.TextValue()
-					if err != nil {
-						return err
-					}
-
-					dt, ok = m.TypeForOID(pgtype.TextOID)
-					if ok {
-						arg = t
-					}
-				}
-			}
-			if !ok {
-				var dv driver.Valuer
-				if dv, ok = arg.(driver.Valuer); ok {
-					v, err := dv.Value()
-					if err != nil {
-						return err
-					}
-					dt, ok = m.TypeForValue(v)
-					if ok {
-						arg = v
-					}
-				}
-			}
-			if !ok {
-				var str fmt.Stringer
-				if str, ok = arg.(fmt.Stringer); ok {
-					dt, ok = m.TypeForOID(pgtype.TextOID)
-					if ok {
-						arg = str.String()
-					}
-				}
-			}
-			if !ok {
-				return &unknownArgumentTypeQueryExecModeExecError{arg: arg}
-			}
-			err := eqb.appendParam(m, dt.OID, TextFormatCode, arg)
-			if err != nil {
-				return err
-			}
+		oid, modArg, err := eqb.oidAndArgForQueryExecModeExec(m, arg)
+		if err != nil {
+			return err
+		}
+
+		err = eqb.appendParam(m, oid, pgtype.TextFormatCode, modArg)
+		if err != nil {
+			return err
 		}
 	}
 
 	return nil
 }
+
+func (eqb *ExtendedQueryBuilder) oidAndArgForQueryExecModeExec(m *pgtype.Map, arg any) (uint32, any, error) {
+	if arg == nil {
+		return 0, arg, nil
+	}
+
+	if dt, ok := m.TypeForValue(arg); ok {
+		return dt.OID, arg, nil
+	}
+
+	if textValuer, ok := arg.(pgtype.TextValuer); ok {
+		tv, err := textValuer.TextValue()
+		if err != nil {
+			return 0, nil, err
+		}
+
+		return pgtype.TextOID, tv, nil
+	}
+
+	if dv, ok := arg.(driver.Valuer); ok {
+		v, err := dv.Value()
+		if err != nil {
+			return 0, nil, err
+		}
+
+		if v == nil {
+			return 0, nil, nil
+		}
+
+		if dt, ok := m.TypeForValue(v); ok {
+			return dt.OID, v, nil
+		}
+	}
+
+	if str, ok := arg.(fmt.Stringer); ok {
+		return pgtype.TextOID, str.String(), nil
+	}
+
+	return 0, nil, &unknownArgumentTypeQueryExecModeExecError{arg: arg}
+}

go.mod

@@ -1,6 +1,6 @@
 module github.com/jackc/pgx/v5
 
-go 1.19
+go 1.20
 
 require (
 	github.com/jackc/pgpassfile v1.0.0

internal/anynil/anynil.go

@@ -1,17 +1,45 @@
 package anynil
 
-import "reflect"
+import (
+	"database/sql/driver"
+	"reflect"
+)
 
-// Is returns true if value is any type of nil. e.g. nil or []byte(nil).
+// valuerReflectType is a reflect.Type for driver.Valuer. It has confusing syntax because reflect.TypeOf returns nil
+// when it's argument is a nil interface value. So we use a pointer to the interface and call Elem to get the actual
+// type. Yuck.
+//
+// This can be simplified in Go 1.22 with reflect.TypeFor.
+//
+// var valuerReflectType = reflect.TypeFor[driver.Valuer]()
+var valuerReflectType = reflect.TypeOf((*driver.Valuer)(nil)).Elem()
+
+// Is returns true if value is any type of nil unless it implements driver.Valuer. *T is not considered to implement
+// driver.Valuer if it is only implemented by T.
 func Is(value any) bool {
 	if value == nil {
 		return true
 	}
 
 	refVal := reflect.ValueOf(value)
-	switch refVal.Kind() {
+	kind := refVal.Kind()
+	switch kind {
 	case reflect.Chan, reflect.Func, reflect.Map, reflect.Ptr, reflect.UnsafePointer, reflect.Interface, reflect.Slice:
-		return refVal.IsNil()
+		if !refVal.IsNil() {
+			return false
+		}
+
+		if _, ok := value.(driver.Valuer); ok {
+			if kind == reflect.Ptr {
+				// The type assertion will succeed if driver.Valuer is implemented on T or *T. Check if it is implemented on T
+				// to see if it is not implemented on *T.
+				return refVal.Type().Elem().Implements(valuerReflectType)
+			} else {
+				return false
+			}
+		}
+
+		return true
 	default:
 		return false
 	}

pgtype/doc.go

@@ -139,6 +139,16 @@ Compatibility with database/sql
 pgtype also includes support for custom types implementing the database/sql.Scanner and database/sql/driver.Valuer
 interfaces.
 
+Encoding Typed Nils
+
+pgtype normalizes typed nils (e.g. []byte(nil)) into nil. nil is always encoded is the SQL NULL value without going
+through the Codec system. This means that Codecs and other encoding logic does not have to handle nil or *T(nil).
+
+However, database/sql compatibility requires Value to be called on T(nil) when T implements driver.Valuer. Therefore,
+driver.Valuer values are not normalized to nil unless it is a *T(nil) where driver.Valuer is implemented on T. See
+https://github.com/golang/go/issues/8415 and
+https://github.com/golang/go/commit/0ce1d79a6a771f7449ec493b993ed2a720917870.
+
 Child Records
 
 pgtype's support for arrays and composite records can be used to load records and their children in a single query.  See

query_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"context"
 	"database/sql"
+	"database/sql/driver"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"os"
@@ -1171,6 +1173,161 @@ func TestConnQueryDatabaseSQLDriverValuerWithAutoGeneratedPointerReceiver(t *tes
 	ensureConnValid(t, conn)
 }
 
+type nilPointerAsEmptyJSONObject struct {
+	ID   string
+	Name string
+}
+
+func (v *nilPointerAsEmptyJSONObject) Value() (driver.Value, error) {
+	if v == nil {
+		return "{}", nil
+	}
+
+	return json.Marshal(v)
+}
+
+// https://github.com/jackc/pgx/issues/1566
+func TestConnQueryDatabaseSQLDriverValuerCalledOnNilPointerImplementers(t *testing.T) {
+	t.Parallel()
+
+	conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE"))
+	defer closeConn(t, conn)
+
+	mustExec(t, conn, "create temporary table t(v json not null)")
+
+	var v *nilPointerAsEmptyJSONObject
+	commandTag, err := conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
+	require.NoError(t, err)
+	require.Equal(t, "INSERT 0 1", commandTag.String())
+
+	var s string
+	err = conn.QueryRow(context.Background(), "select v from t").Scan(&s)
+	require.NoError(t, err)
+	require.Equal(t, "{}", s)
+
+	_, err = conn.Exec(context.Background(), `delete from t`)
+	require.NoError(t, err)
+
+	v = &nilPointerAsEmptyJSONObject{ID: "1", Name: "foo"}
+	commandTag, err = conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
+	require.NoError(t, err)
+	require.Equal(t, "INSERT 0 1", commandTag.String())
+
+	var v2 *nilPointerAsEmptyJSONObject
+	err = conn.QueryRow(context.Background(), "select v from t").Scan(&v2)
+	require.NoError(t, err)
+	require.Equal(t, v, v2)
+
+	ensureConnValid(t, conn)
+}
+
+type nilSliceAsEmptySlice []byte
+
+func (j nilSliceAsEmptySlice) Value() (driver.Value, error) {
+	if len(j) == 0 {
+		return []byte("[]"), nil
+	}
+
+	return []byte(j), nil
+}
+
+func (j *nilSliceAsEmptySlice) UnmarshalJSON(data []byte) error {
+	*j = bytes.Clone(data)
+	return nil
+}
+
+// https://github.com/jackc/pgx/issues/1860
+func TestConnQueryDatabaseSQLDriverValuerCalledOnNilSliceImplementers(t *testing.T) {
+	t.Parallel()
+
+	conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE"))
+	defer closeConn(t, conn)
+
+	mustExec(t, conn, "create temporary table t(v json not null)")
+
+	var v nilSliceAsEmptySlice
+	commandTag, err := conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
+	require.NoError(t, err)
+	require.Equal(t, "INSERT 0 1", commandTag.String())
+
+	var s string
+	err = conn.QueryRow(context.Background(), "select v from t").Scan(&s)
+	require.NoError(t, err)
+	require.Equal(t, "[]", s)
+
+	_, err = conn.Exec(context.Background(), `delete from t`)
+	require.NoError(t, err)
+
+	v = nilSliceAsEmptySlice(`{"name": "foo"}`)
+	commandTag, err = conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
+	require.NoError(t, err)
+	require.Equal(t, "INSERT 0 1", commandTag.String())
+
+	var v2 nilSliceAsEmptySlice
+	err = conn.QueryRow(context.Background(), "select v from t").Scan(&v2)
+	require.NoError(t, err)
+	require.Equal(t, v, v2)
+
+	ensureConnValid(t, conn)
+}
+
+type nilMapAsEmptyObject map[string]any
+
+func (j nilMapAsEmptyObject) Value() (driver.Value, error) {
+	if j == nil {
+		return []byte("{}"), nil
+	}
+
+	return json.Marshal(j)
+}
+
+func (j *nilMapAsEmptyObject) UnmarshalJSON(data []byte) error {
+	var m map[string]any
+	err := json.Unmarshal(data, &m)
+	if err != nil {
+		return err
+	}
+
+	*j = m
+
+	return nil
+}
+
+// https://github.com/jackc/pgx/pull/2019#discussion_r1605806751
+func TestConnQueryDatabaseSQLDriverValuerCalledOnNilMapImplementers(t *testing.T) {
+	t.Parallel()
+
+	conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE"))
+	defer closeConn(t, conn)
+
+	mustExec(t, conn, "create temporary table t(v json not null)")
+
+	var v nilMapAsEmptyObject
+	commandTag, err := conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
+	require.NoError(t, err)
+	require.Equal(t, "INSERT 0 1", commandTag.String())
+
+	var s string
+	err = conn.QueryRow(context.Background(), "select v from t").Scan(&s)
+	require.NoError(t, err)
+	require.Equal(t, "{}", s)
+
+	_, err = conn.Exec(context.Background(), `delete from t`)
+	require.NoError(t, err)
+
+	v = nilMapAsEmptyObject{"name": "foo"}
+	commandTag, err = conn.Exec(context.Background(), `insert into t(v) values($1)`, v)
+	require.NoError(t, err)
+	require.Equal(t, "INSERT 0 1", commandTag.String())
+
+	var v2 nilMapAsEmptyObject
+	err = conn.QueryRow(context.Background(), "select v from t").Scan(&v2)
+	require.NoError(t, err)
+	require.Equal(t, v, v2)
+
+	ensureConnValid(t, conn)
+}
+
 func TestConnQueryDatabaseSQLDriverScannerWithBinaryPgTypeThatAcceptsSameType(t *testing.T) {
 	t.Parallel()