Reduce SQL sanitizer allocations

CHANGELOG.md

@@ -1,3 +1,25 @@
+# 5.7.1 (September 10, 2024)
+
+* Fix data race in tracelog.TraceLog
+* Update puddle to v2.2.2. This removes the import of nanotime via linkname.
+* Update golang.org/x/crypto and golang.org/x/text
+
+# 5.7.0 (September 7, 2024)
+
+* Add support for sslrootcert=system (Yann Soubeyrand)
+* Add LoadTypes to load multiple types in a single SQL query (Nick Farrell)
+* Add XMLCodec supports encoding + scanning XML column type like json (nickcruess-soda)
+* Add MultiTrace (Stepan Rabotkin)
+* Add TraceLogConfig with customizable TimeKey (stringintech)
+* pgx.ErrNoRows wraps sql.ErrNoRows to aid in database/sql compatibility with native pgx functions (merlin)
+* Support scanning binary formatted uint32 into string / TextScanner (jennifersp)
+* Fix interval encoding to allow 0s and avoid extra spaces (Carlos Pérez-Aradros Herce)
+* Update pgservicefile - fixes panic when parsing invalid file
+* Better error message when reading past end of batch
+* Don't print url when url.Parse returns an error (Kevin Biju)
+* Fix snake case name normalization collision in RowToStructByName with db tag (nolandseigler)
+* Fix: Scan and encode types with underlying types of arrays
+
 # 5.6.0 (May 25, 2024)
 
 * Add StrictNamedArgs (Tomas Zahradnicek)

batch_test.go

@@ -1008,6 +1008,36 @@ func TestSendBatchSimpleProtocol(t *testing.T) {
 	assert.False(t, rows.Next())
 }
 
+// https://github.com/jackc/pgx/issues/1847#issuecomment-2347858887
+func TestConnSendBatchErrorDoesNotLeaveOrphanedPreparedStatement(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+	defer cancel()
+
+	pgxtest.RunWithQueryExecModes(ctx, t, defaultConnTestRunner, nil, func(ctx context.Context, t testing.TB, conn *pgx.Conn) {
+		pgxtest.SkipCockroachDB(t, conn, "Server serial type is incompatible with test")
+
+		mustExec(t, conn, `create temporary table foo(col1 text primary key);`)
+
+		batch := &pgx.Batch{}
+		batch.Queue("select col1 from foo")
+		batch.Queue("select col1 from baz")
+		err := conn.SendBatch(ctx, batch).Close()
+		require.EqualError(t, err, `ERROR: relation "baz" does not exist (SQLSTATE 42P01)`)
+
+		mustExec(t, conn, `create temporary table baz(col1 text primary key);`)
+
+		// Since table baz now exists, the batch should succeed.
+
+		batch = &pgx.Batch{}
+		batch.Queue("select col1 from foo")
+		batch.Queue("select col1 from baz")
+		err = conn.SendBatch(ctx, batch).Close()
+		require.NoError(t, err)
+	})
+}
+
 func ExampleConn_SendBatch() {
 	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
 	defer cancel()

conn.go

@@ -3,6 +3,7 @@ package pgx
 import (
 	"context"
 	"crypto/sha256"
+	"database/sql"
 	"encoding/hex"
 	"errors"
 	"fmt"
@@ -102,11 +103,27 @@ func (ident Identifier) Sanitize() string {
 
 var (
 	// ErrNoRows occurs when rows are expected but none are returned.
-	ErrNoRows = errors.New("no rows in result set")
+	ErrNoRows = newProxyErr(sql.ErrNoRows, "no rows in result set")
 	// ErrTooManyRows occurs when more rows than expected are returned.
 	ErrTooManyRows = errors.New("too many rows in result set")
 )
 
+func newProxyErr(background error, msg string) error {
+	return &proxyError{
+		msg:        msg,
+		background: background,
+	}
+}
+
+type proxyError struct {
+	msg        string
+	background error
+}
+
+func (err *proxyError) Error() string { return err.msg }
+
+func (err *proxyError) Unwrap() error { return err.background }
+
 var (
 	errDisabledStatementCache   = fmt.Errorf("cannot use QueryExecModeCacheStatement with disabled statement cache")
 	errDisabledDescriptionCache = fmt.Errorf("cannot use QueryExecModeCacheDescribe with disabled description cache")
@@ -1109,47 +1126,64 @@ func (c *Conn) sendBatchExtendedWithDescription(ctx context.Context, b *Batch, d
 
 	// Prepare any needed queries
 	if len(distinctNewQueries) > 0 {
-		for _, sd := range distinctNewQueries {
-			pipeline.SendPrepare(sd.Name, sd.SQL, nil)
-		}
+		err := func() (err error) {
+			for _, sd := range distinctNewQueries {
+				pipeline.SendPrepare(sd.Name, sd.SQL, nil)
+			}
 
-		err := pipeline.Sync()
-		if err != nil {
-			return &pipelineBatchResults{ctx: ctx, conn: c, err: err, closed: true}
-		}
+			// Store all statements we are preparing into the cache. It's fine if it overflows because HandleInvalidated will
+			// clean them up later.
+			if sdCache != nil {
+				for _, sd := range distinctNewQueries {
+					sdCache.Put(sd)
+				}
+			}
+
+			// If something goes wrong preparing the statements, we need to invalidate the cache entries we just added.
+			defer func() {
+				if err != nil && sdCache != nil {
+					for _, sd := range distinctNewQueries {
+						sdCache.Invalidate(sd.SQL)
+					}
+				}
+			}()
+
+			err = pipeline.Sync()
+			if err != nil {
+				return err
+			}
+
+			for _, sd := range distinctNewQueries {
+				results, err := pipeline.GetResults()
+				if err != nil {
+					return err
+				}
+
+				resultSD, ok := results.(*pgconn.StatementDescription)
+				if !ok {
+					return fmt.Errorf("expected statement description, got %T", results)
+				}
+
+				// Fill in the previously empty / pending statement descriptions.
+				sd.ParamOIDs = resultSD.ParamOIDs
+				sd.Fields = resultSD.Fields
+			}
 
-		for _, sd := range distinctNewQueries {
 			results, err := pipeline.GetResults()
 			if err != nil {
-				return &pipelineBatchResults{ctx: ctx, conn: c, err: err, closed: true}
+				return err
 			}
 
-			resultSD, ok := results.(*pgconn.StatementDescription)
+			_, ok := results.(*pgconn.PipelineSync)
 			if !ok {
-				return &pipelineBatchResults{ctx: ctx, conn: c, err: fmt.Errorf("expected statement description, got %T", results), closed: true}
+				return fmt.Errorf("expected sync, got %T", results)
 			}
 
-			// Fill in the previously empty / pending statement descriptions.
-			sd.ParamOIDs = resultSD.ParamOIDs
-			sd.Fields = resultSD.Fields
-		}
-
-		results, err := pipeline.GetResults()
+			return nil
+		}()
 		if err != nil {
 			return &pipelineBatchResults{ctx: ctx, conn: c, err: err, closed: true}
 		}
-
-		_, ok := results.(*pgconn.PipelineSync)
-		if !ok {
-			return &pipelineBatchResults{ctx: ctx, conn: c, err: fmt.Errorf("expected sync, got %T", results), closed: true}
-		}
-	}
-
-	// Put all statements into the cache. It's fine if it overflows because HandleInvalidated will clean them up later.
-	if sdCache != nil {
-		for _, sd := range distinctNewQueries {
-			sdCache.Put(sd)
-		}
 	}
 
 	// Queue the queries.

conn_test.go

@@ -3,6 +3,7 @@ package pgx_test
 import (
 	"bytes"
 	"context"
+	"database/sql"
 	"os"
 	"strings"
 	"sync"
@@ -1408,3 +1409,13 @@ func TestConnDeallocateInvalidatedCachedStatementsInTransactionWithBatch(t *test
 
 	ensureConnValid(t, conn)
 }
+
+func TestErrNoRows(t *testing.T) {
+	t.Parallel()
+
+	// ensure we preserve old error message
+	require.Equal(t, "no rows in result set", pgx.ErrNoRows.Error())
+
+	require.ErrorIs(t, pgx.ErrNoRows, sql.ErrNoRows, "pgx.ErrNowRows must match sql.ErrNoRows")
+	require.ErrorIs(t, pgx.ErrNoRows, pgx.ErrNoRows, "sql.ErrNowRows must match pgx.ErrNoRows")
+}

go.mod

@@ -5,17 +5,17 @@ go 1.21
 require (
 	github.com/jackc/pgpassfile v1.0.0
 	github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761
-	github.com/jackc/puddle/v2 v2.2.1
+	github.com/jackc/puddle/v2 v2.2.2
 	github.com/stretchr/testify v1.8.1
-	golang.org/x/crypto v0.17.0
-	golang.org/x/text v0.14.0
+	golang.org/x/crypto v0.27.0
+	golang.org/x/sync v0.8.0
+	golang.org/x/text v0.18.0
 )
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/kr/pretty v0.3.0 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	golang.org/x/sync v0.1.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -6,8 +6,8 @@ github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsI
 github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg=
 github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo=
 github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM=
-github.com/jackc/puddle/v2 v2.2.1 h1:RhxXJtFG022u4ibrCSMSiu5aOq1i77R3OHKNJj77OAk=
-github.com/jackc/puddle/v2 v2.2.1/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
+github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo=
+github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
 github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
@@ -29,12 +29,12 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
-golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k=
-golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4=
-golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
-golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
-golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
+golang.org/x/crypto v0.27.0 h1:GXm2NjJrPaiv/h1tb2UH8QfgC/hOf/+z0p6PT8o1w7A=
+golang.org/x/crypto v0.27.0/go.mod h1:1Xngt8kV6Dvbssa53Ziq6Eqn0HqbZi5Z6R0ZpwQzt70=
+golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
+golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224=
+golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=

internal/sanitize/benchmmark.sh

@@ -0,0 +1,60 @@
+#!/usr/bin/env bash
+
+current_branch=$(git rev-parse --abbrev-ref HEAD)
+if [ "$current_branch" == "HEAD" ]; then
+    current_branch=$(git rev-parse HEAD)
+fi
+
+restore_branch() {
+    echo "Restoring original branch/commit: $current_branch"
+    git checkout "$current_branch"
+}
+trap restore_branch EXIT
+
+# Check if there are uncommitted changes
+if ! git diff --quiet || ! git diff --cached --quiet; then
+    echo "There are uncommitted changes. Please commit or stash them before running this script."
+    exit 1
+fi
+
+# Ensure that at least one commit argument is passed
+if [ "$#" -lt 1 ]; then
+    echo "Usage: $0 <commit1> <commit2> ... <commitN>"
+    exit 1
+fi
+
+commits=("$@")
+benchmarks_dir=benchmarks
+
+if ! mkdir -p "${benchmarks_dir}"; then
+    echo "Unable to create dir for benchmarks data"
+    exit 1
+fi
+
+# Benchmark results
+bench_files=()
+
+# Run benchmark for each listed commit
+for i in "${!commits[@]}"; do
+    commit="${commits[i]}"
+    git checkout "$commit" || {
+        echo "Failed to checkout $commit"
+        exit 1
+    }
+
+    # Sanitized commmit message
+    commit_message=$(git log -1 --pretty=format:"%s" | tr -c '[:alnum:]-_' '_')
+
+    # Benchmark data will go there
+    bench_file="${benchmarks_dir}/${i}_${commit_message}.bench"
+
+    if ! go test -bench=. -count=10 >"$bench_file"; then
+        echo "Benchmarking failed for commit $commit"
+        exit 1
+    fi
+
+    bench_files+=("$bench_file")
+done
+
+# go install golang.org/x/perf/cmd/benchstat[@latest]
+benchstat "${bench_files[@]}"