diff --git a/CHANGELOG.md b/CHANGELOG.md index 62539608a..e44f95f4d 100644 --- a/CHANGELOG.md +++ b/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) -------------------- diff --git a/conn.go b/conn.go index fc3110942..4e5eb3c45 100644 --- a/conn.go +++ b/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) diff --git a/hstore/hstore.go b/hstore/hstore.go index 7983872ba..af9fa6dd3 100644 --- a/hstore/hstore.go +++ b/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 } diff --git a/hstore/hstore_test.go b/hstore/hstore_test.go index 1f9b47bc1..98ec6842b 100644 --- a/hstore/hstore_test.go +++ b/hstore/hstore_test.go @@ -2,6 +2,8 @@ package hstore import ( "database/sql" + "reflect" + "strings" "testing" _ "github.com/lib/pq" @@ -9,120 +11,93 @@ import ( ) 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", + "<>"=>"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}, + "<>": {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}, - "<>": {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}, + "<>": {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() + } + }) }