Check for BinaryValue to allow types to use a different binary encoding

CHANGELOG.md

@@ -14,6 +14,8 @@ unreleased
 
 - Fix SSL key permission check to allow modes stricter than 0600/0640#1265 ([#1265]).
 
+- Fix Hstore to work with binary parameters ([#1278]).
+
 - Clearer error when starting a new query while pq is still processing another
   query ([#1272]).
 
@@ -26,6 +28,7 @@ unreleased
 [#1270]: https://github.com/lib/pq/pull/1270
 [#1272]: https://github.com/lib/pq/pull/1272
 [#1277]: https://github.com/lib/pq/pull/1277
+[#1278]: https://github.com/lib/pq/pull/1278
 
 v1.11.2 (2026-02-10)
 --------------------

conn.go

@@ -834,6 +834,14 @@ func toNamedValue(v []driver.Value) []driver.NamedValue {
 
 // CheckNamedValue implements [driver.NamedValueChecker].
 func (cn *conn) CheckNamedValue(nv *driver.NamedValue) error {
+	if cn.cfg.BinaryParameters {
+		if bin, ok := nv.Value.(interface{ BinaryValue() ([]byte, error) }); ok {
+			var err error
+			nv.Value, err = bin.BinaryValue()
+			return err
+		}
+	}
+
 	// Ignore Valuer, for backward compatibility with pq.Array().
 	if _, ok := nv.Value.(driver.Valuer); ok {
 		return driver.ErrSkip
@@ -1474,7 +1482,7 @@ func md5s(s string) string {
 
 func (cn *conn) sendBinaryParameters(b *writeBuf, args []driver.NamedValue) error {
 	// Do one pass over the parameters to see if we're going to send any of them
-	// over in binary.  If we are, create a paramFormats array at the same time.
+	// over in binary. If we are, create a paramFormats array at the same time.
 	var paramFormats []int
 	for i, x := range args {
 		_, ok := x.Value.([]byte)

hstore/hstore.go

@@ -1,8 +1,11 @@
 package hstore
 
 import (
+	"bytes"
 	"database/sql"
 	"database/sql/driver"
+	"encoding/binary"
+	"math"
 	"strings"
 )
 
@@ -11,25 +14,6 @@ type Hstore struct {
 	Map map[string]sql.NullString
 }
 
-// Escapes and quotes hstore keys/values. s should be a sql.NullString or string
-func hQuote(s any) string {
-	var str string
-	switch v := s.(type) {
-	case sql.NullString:
-		if !v.Valid {
-			return "NULL"
-		}
-		str = v.String
-	case string:
-		str = v
-	default:
-		panic("not a string or sql.NullString")
-	}
-
-	str = strings.Replace(str, "\\", "\\\\", -1)
-	return `"` + strings.Replace(str, "\"", "\\\"", -1) + `"`
-}
-
 // Scan implements the Scanner interface.
 //
 // Note h.Map is reallocated before the scan to clear existing values. If the
@@ -102,16 +86,68 @@ func (h *Hstore) Scan(value any) error {
 	return nil
 }
 
+var hQuoteRepl = strings.NewReplacer("\\", "\\\\", "\"", "\\\"")
+
+// Escapes and quotes hstore keys/values. s should be a sql.NullString or string
+func hQuote(b *bytes.Buffer, s any) {
+	var str string
+	switch v := s.(type) {
+	case sql.NullString:
+		if !v.Valid {
+			b.WriteString("NULL")
+			return
+		}
+		str = v.String
+	case string:
+		str = v
+	default:
+		panic("not a string or sql.NullString")
+	}
+
+	b.WriteByte('"')
+	b.WriteString(hQuoteRepl.Replace(str))
+	b.WriteByte('"')
+}
+
 // Value implements the driver Valuer interface. Note if h.Map is nil, the
 // database column value will be set to NULL.
 func (h Hstore) Value() (driver.Value, error) {
 	if h.Map == nil {
 		return nil, nil
 	}
-	parts := []string{}
-	for key, val := range h.Map {
-		thispart := hQuote(key) + "=>" + hQuote(val)
-		parts = append(parts, thispart)
+	if len(h.Map) == 0 {
+		return []byte(""), nil
+	}
+
+	b := new(bytes.Buffer)
+	b.Grow(len(h.Map) * 8)
+	for k, v := range h.Map {
+		if b.Len() > 0 {
+			b.WriteByte(',')
+		}
+		hQuote(b, k)
+		b.WriteString("=>")
+		hQuote(b, v)
+	}
+	return b.Bytes(), nil
+}
+
+func (h Hstore) BinaryValue() ([]byte, error) {
+	if h.Map == nil {
+		return nil, nil
+	}
+
+	b := make([]byte, 0, len(h.Map)*12)
+	b = binary.BigEndian.AppendUint32(b, uint32(len(h.Map)))
+	for k, v := range h.Map {
+		b = binary.BigEndian.AppendUint32(b, uint32(len(k)))
+		b = append(b, k...)
+		if v.Valid {
+			b = binary.BigEndian.AppendUint32(b, uint32(len(v.String)))
+			b = append(b, v.String...)
+		} else {
+			b = binary.BigEndian.AppendUint32(b, math.MaxUint32)
+		}
 	}
-	return []byte(strings.Join(parts, ",")), nil
+	return b, nil
 }

hstore/hstore_test.go

@@ -2,127 +2,102 @@ package hstore
 
 import (
 	"database/sql"
+	"reflect"
+	"strings"
 	"testing"
 
 	_ "github.com/lib/pq"
 	"github.com/lib/pq/internal/pqtest"
 )
 
 func TestHstore(t *testing.T) {
-	if pqtest.ForceBinaryParameters() {
-		t.Skip("Currently fails with PQTEST_BINARY_PARAMETERS=1") // TODO
-	}
-
-	db := pqtest.MustDB(t)
-
-	// quietly create hstore if it doesn't exist
-	_, err := db.Exec("CREATE EXTENSION IF NOT EXISTS hstore")
-	if err != nil {
-		t.Skipf("Skipping hstore tests - hstore extension create failed: %s", err.Error())
-	}
-
-	hs := Hstore{}
-
-	// test for null-valued hstores
-	err = db.QueryRow("SELECT NULL::hstore").Scan(&hs)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if hs.Map != nil {
-		t.Fatalf("expected null map")
-	}
-
-	err = db.QueryRow("SELECT $1::hstore", hs).Scan(&hs)
-	if err != nil {
-		t.Fatalf("re-query null map failed: %s", err.Error())
-	}
-	if hs.Map != nil {
-		t.Fatalf("expected null map")
-	}
-
-	// test for empty hstores
-	err = db.QueryRow("SELECT ''::hstore").Scan(&hs)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if hs.Map == nil {
-		t.Fatalf("expected empty map, got null map")
-	}
-	if len(hs.Map) != 0 {
-		t.Fatalf("expected empty map, got len(map)=%d", len(hs.Map))
-	}
-
-	err = db.QueryRow("SELECT $1::hstore", hs).Scan(&hs)
-	if err != nil {
-		t.Fatalf("re-query empty map failed: %s", err.Error())
-	}
-
-	if hs.Map == nil {
-		t.Fatalf("expected empty map, got null map")
-	}
-	if len(hs.Map) != 0 {
-		t.Fatalf("expected empty map, got len(map)=%d", len(hs.Map))
-	}
-
-	// a few example maps to test out
-	hsOnePair := Hstore{
-		Map: map[string]sql.NullString{
+	tr := strings.NewReplacer("\t", "", "\n", "", `\n`, "\n", `\t`, "\t")
+	tests := []struct {
+		in   string
+		want Hstore
+	}{
+		{`null`, Hstore{}},
+		{`''`, Hstore{Map: map[string]sql.NullString{}}},
+
+		{`'"key1"=>"value1"'`, Hstore{Map: map[string]sql.NullString{
 			"key1": {String: "value1", Valid: true},
-		},
-	}
-
-	hsThreePairs := Hstore{
-		Map: map[string]sql.NullString{
+		}}},
+		{`'"key1"=>"value1","key2"=>"value2","key3"=>"value3"'`, Hstore{Map: map[string]sql.NullString{
 			"key1": {String: "value1", Valid: true},
 			"key2": {String: "value2", Valid: true},
 			"key3": {String: "value3", Valid: true},
-		},
-	}
+		}}},
+		{
+			tr.Replace(`'
+				"embedded2"=>"\"value2\"=>x2",
+				"withnewlines"=>"\n\nvalue\t=>2",
+				"nullstring"=>"NULL",
+				"withbracket"=>"value>42",
+				"\"withquotes1\""=>"this \"should\" be fine",
+				"embedded1"=>"value1=>x1",
+				"<<all sorts of crazy>>"=>"this, \"should,\\\" also, => be fine",
+				"actuallynull"=>NULL,
+				"NULL"=>"NULL string key",
+				"withequal"=>"value=42",
+				"\"withquotes\"2\""=>"this \"should\\\" also be fine"
+			'`),
+			Hstore{Map: map[string]sql.NullString{
+				"nullstring":             {String: "NULL", Valid: true},
+				"actuallynull":           {String: "", Valid: false},
+				"NULL":                   {String: "NULL string key", Valid: true},
+				"withbracket":            {String: "value>42", Valid: true},
+				"withequal":              {String: "value=42", Valid: true},
+				`"withquotes1"`:          {String: `this "should" be fine`, Valid: true},
+				`"withquotes"2"`:         {String: `this "should\" also be fine`, Valid: true},
+				"embedded1":              {String: "value1=>x1", Valid: true},
+				"embedded2":              {String: `"value2"=>x2`, Valid: true},
+				"withnewlines":           {String: "\n\nvalue\t=>2", Valid: true},
+				"<<all sorts of crazy>>": {String: `this, "should,\" also, => be fine`, Valid: true},
+			}}},
+	}
+
+	t.Parallel()
+	db := pqtest.MustDB(t)
+	pqtest.Exec(t, db, `create extension if not exists hstore`)
+	for _, tt := range tests {
+		t.Run("", func(t *testing.T) {
+			have := pqtest.Query[Hstore](t, db, `select `+tt.in+`::hstore as hstore`)[0]["hstore"]
+			if !reflect.DeepEqual(have, tt.want) {
+				t.Fatalf("\nhave: %#v\nwant: %#v", have, tt.want)
+			}
 
-	hsSmorgasbord := Hstore{
-		Map: map[string]sql.NullString{
-			"nullstring":             {String: "NULL", Valid: true},
-			"actuallynull":           {String: "", Valid: false},
-			"NULL":                   {String: "NULL string key", Valid: true},
-			"withbracket":            {String: "value>42", Valid: true},
-			"withequal":              {String: "value=42", Valid: true},
-			`"withquotes1"`:          {String: `this "should" be fine`, Valid: true},
-			`"withquotes"2"`:         {String: `this "should\" also be fine`, Valid: true},
-			"embedded1":              {String: "value1=>x1", Valid: true},
-			"embedded2":              {String: `"value2"=>x2`, Valid: true},
-			"withnewlines":           {String: "\n\nvalue\t=>2", Valid: true},
-			"<<all sorts of crazy>>": {String: `this, "should,\" also, => be fine`, Valid: true},
-		},
+			have2 := pqtest.Query[Hstore](t, db, `select $1::hstore as hstore`, have)[0]["hstore"]
+			if !reflect.DeepEqual(have2, tt.want) {
+				t.Errorf("\nhave: %#v\nwant: %#v", have2, tt.want)
+			}
+		})
 	}
+}
 
-	// test encoding in query params, then decoding during Scan
-	testBidirectional := func(h Hstore) {
-		err = db.QueryRow("SELECT $1::hstore", h).Scan(&hs)
-		if err != nil {
-			t.Fatalf("re-query %d-pair map failed: %s", len(h.Map), err.Error())
-		}
-		if hs.Map == nil {
-			t.Fatalf("expected %d-pair map, got null map", len(h.Map))
-		}
-		if len(hs.Map) != len(h.Map) {
-			t.Fatalf("expected %d-pair map, got len(map)=%d", len(h.Map), len(hs.Map))
-		}
+func BenchmarkHstore(b *testing.B) {
+	h := Hstore{Map: map[string]sql.NullString{
+		"nullstring":             {String: "NULL", Valid: true},
+		"actuallynull":           {String: "", Valid: false},
+		"NULL":                   {String: "NULL string key", Valid: true},
+		"withbracket":            {String: "value>42", Valid: true},
+		"withequal":              {String: "value=42", Valid: true},
+		`"withquotes1"`:          {String: `this "should" be fine`, Valid: true},
+		`"withquotes"2"`:         {String: `this "should\" also be fine`, Valid: true},
+		"embedded1":              {String: "value1=>x1", Valid: true},
+		"embedded2":              {String: `"value2"=>x2`, Valid: true},
+		"withnewlines":           {String: "\n\nvalue\t=>2", Valid: true},
+		"<<all sorts of crazy>>": {String: `this, "should,\" also, => be fine`, Valid: true},
+	}}
+
+	b.Run("Value", func(b *testing.B) {
+		for i := 0; i < b.N; i++ {
+			_, _ = h.Value()
 
-		for key, val := range hs.Map {
-			otherval, found := h.Map[key]
-			if !found {
-				t.Fatalf("  key '%v' not found in %d-pair map", key, len(h.Map))
-			}
-			if otherval.Valid != val.Valid {
-				t.Fatalf("  value %v <> %v in %d-pair map", otherval, val, len(h.Map))
-			}
-			if otherval.String != val.String {
-				t.Fatalf("  value '%v' <> '%v' in %d-pair map", otherval.String, val.String, len(h.Map))
-			}
 		}
-	}
-
-	testBidirectional(hsOnePair)
-	testBidirectional(hsThreePairs)
-	testBidirectional(hsSmorgasbord)
+	})
+	b.Run("BinaryValue", func(b *testing.B) {
+		for i := 0; i < b.N; i++ {
+			_, _ = h.BinaryValue()
+		}
+	})
 }