refactor(dbexec): 🔧 improve error handling in QueryRow and Scan

* change `rowWithAfter` to use a pointer receiver for `Scan`
* ensure callback is only called once using `sync.Once`
* update tests to reflect changes in `Scan` behavior
* enhance query builder to quote identifiers for safety

Author: mberbero

README.md

@@ -2,6 +2,8 @@
 
 Production-ready, lightweight ORM and query builder for PostgreSQL on top of PGX v5. Ships with connection pooling, automatic migrations from struct tags, a fluent query builder, generic repository, soft delete, optimistic locking, transactions, read/write splitting, retry/backoff, a circuit breaker, and comprehensive e2e tests.
 
+`QueryBuilder` instances are mutable and not goroutine-safe. Create a fresh builder per query chain.
+
 ### Features
 
 - Fast, reliable connections via PGX v5 (`pgxpool`)
@@ -18,6 +20,8 @@ Production-ready, lightweight ORM and query builder for PostgreSQL on top of PGX
 
 Note: OpenTelemetry/Prometheus integrations are not included yet.
 
+Safety note: `Table`, `Join`, `OrderBy`, `Set`, and `Raw` accept SQL fragments and should only receive trusted application SQL. Identifier-oriented helpers like `TableQ`, `SelectQ`, and repository reflection paths quote identifiers automatically, but values should still be passed through placeholders.
+
 ### Install
 
 ```bash

bench_test.go

@@ -76,7 +76,7 @@ func BenchmarkQueryBuilderKeysetPredicate(b *testing.B) {
 			OrderBy("id ASC").
 			After("id", 100).
 			Before("id", 1000)
-		_ = qb.buildKeysetPredicate()
+		_, _ = qb.buildKeysetPredicate(0)
 	}
 }
 

coverage.out

@@ -2,6 +2,7 @@ package norm
 
 import (
 	"context"
+	"sync"
 
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgconn"
@@ -51,7 +52,7 @@ func (b breakerExecuter) QueryRow(ctx context.Context, sql string, args ...any)
 			return errorRow{err: err}
 		}
 		row := b.exec.QueryRow(ctx, sql, args...)
-		return rowWithAfter{Row: row, after: func(err error) { br.after(err) }}
+		return &rowWithAfter{Row: row, after: func(err error) { br.after(err) }}
 	}
 	return b.exec.QueryRow(ctx, sql, args...)
 }
@@ -65,13 +66,16 @@ func (e errorRow) Scan(dest ...any) error { return e.err }
 type rowWithAfter struct {
 	pgx.Row
 	after func(error)
+	once  sync.Once
 }
 
-func (r rowWithAfter) Scan(dest ...any) error {
+func (r *rowWithAfter) Scan(dest ...any) error {
 	err := r.Row.Scan(dest...)
-	if r.after != nil {
-		r.after(err)
-	}
+	r.once.Do(func() {
+		if r.after != nil {
+			r.after(err)
+		}
+	})
 	return err
 }
 
@@ -111,7 +115,7 @@ func (r routingExecuter) QueryRow(ctx context.Context, sql string, args ...any)
 			return errorRow{err: err}
 		}
 		row := exec.QueryRow(ctx, sql, args...)
-		return rowWithAfter{Row: row, after: func(err error) { br.after(err) }}
+		return &rowWithAfter{Row: row, after: func(err error) { br.after(err) }}
 	}
 	return exec.QueryRow(ctx, sql, args...)
 }

dbexec.go

@@ -12,9 +12,10 @@ func (f *fakeBaseRow) Scan(_ ...any) error { f.scanned++; return f.retErr }
 func TestRowWithAfter_CallsCallback(t *testing.T) {
 	base := &fakeBaseRow{}
 	called := 0
-	r := rowWithAfter{Row: base, after: func(err error) { called++ }}
+	r := &rowWithAfter{Row: base, after: func(err error) { called++ }}
 	_ = r.Scan()
-	if called != 1 || base.scanned != 1 {
+	_ = r.Scan()
+	if called != 1 || base.scanned != 2 {
 		t.Fatalf("after not called or scan not forwarded")
 	}
 }

dbexec_row_after_test.go

@@ -1,5 +1,9 @@
 ## Query Builder
 
+`QueryBuilder` is mutable and not goroutine-safe. Build a fresh chain per query instead of reusing the same builder across concurrent goroutines.
+
+`Table`, `Join`, `OrderBy`, `Set`, and `Raw` accept SQL fragments. Do not pass untrusted user input to these methods. Use placeholders for values, and prefer `TableQ` / `SelectQ` / `SelectQI` when you need identifier quoting.
+
 Select:
 
 ```go
@@ -37,6 +41,8 @@ Keyset pagination helpers:
 _ = db.Query().Table("users").OrderBy("id ASC").After("id", 123).Limit(20).Find(ctx, &rows)
 ```
 
+`After` / `Before`, `Insert`, `Returning`, and `OnConflict` are intended for column identifiers and are quoted automatically.
+
 Read routing:
 
 ```go

docs/guides/query-builder.md

@@ -2,6 +2,8 @@
 
 Generic CRUD with soft delete, pagination, upsert, and bulk insert via CopyFrom.
 
+`UpdatePartial`, `Upsert`, `CreateCopyFrom`, and struct-tag-driven CRUD APIs treat column names as identifiers. Keep those names static in application code; values should still come from placeholders or map values, not string-built SQL.
+
 ```go
 type User struct { /* fields with db/norm tags */ }
 

docs/guides/repository.md

@@ -0,0 +1,73 @@
+package norm
+
+import (
+	"bytes"
+	"log"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestStdLoggerAndFormatHelpers(t *testing.T) {
+	oldWriter := log.Writer()
+	oldFlags := log.Flags()
+	defer log.SetOutput(oldWriter)
+	defer log.SetFlags(oldFlags)
+
+	var buf bytes.Buffer
+	log.SetOutput(&buf)
+	log.SetFlags(0)
+
+	StdLogger{}.Debug("query", Field{Key: "id", Value: 7}, Field{Key: "name", Value: "alice"})
+	out := strings.TrimSpace(buf.String())
+	if !strings.Contains(out, "[DEBUG] query id=7 name=alice") {
+		t.Fatalf("unexpected log output: %q", out)
+	}
+
+	buf.Reset()
+	StdLogger{}.Info("ignored", Field{Key: "stmt", Value: "SELECT 1;"})
+	if got := strings.TrimSpace(buf.String()); got != "SELECT 1;" {
+		t.Fatalf("stmt shortcut mismatch: %q", got)
+	}
+
+	if got := formatFields([]Field{{Key: "n", Value: 3}, {Key: "s", Value: "x"}}); got != "n=3 s=x" {
+		t.Fatalf("formatFields=%q", got)
+	}
+	if got := formatFields(nil); got != "" {
+		t.Fatalf("expected empty fields, got %q", got)
+	}
+}
+
+func TestInlineSQLAndSQLLiteral(t *testing.T) {
+	ts := time.Unix(1700000000, 0).UTC()
+	got := inlineSQL("INSERT INTO t VALUES ($1, $2, $3, $4, $5, $6)", []any{"O'Reilly", []byte{0xAB, 0xCD}, true, nil, ts, 7})
+	checks := []string{
+		"'O''Reilly'",
+		"decode('ABCD','hex')",
+		"TRUE",
+		"NULL",
+		ts.Format(time.RFC3339Nano),
+		"7",
+	}
+	for _, check := range checks {
+		if !strings.Contains(got, check) {
+			t.Fatalf("inlineSQL missing %q in %q", check, got)
+		}
+	}
+	if !strings.HasSuffix(got, ";") {
+		t.Fatalf("inlineSQL should end with semicolon: %q", got)
+	}
+
+	if got := inlineSQL("SELECT 1", nil); got != "SELECT 1;" {
+		t.Fatalf("inlineSQL no-args mismatch: %q", got)
+	}
+	if got := sqlLiteral(false); got != "FALSE" {
+		t.Fatalf("sqlLiteral false mismatch: %q", got)
+	}
+	if got := sqlLiteral(struct{ Name string }{Name: "bob"}); !strings.Contains(got, "bob") {
+		t.Fatalf("sqlLiteral struct mismatch: %q", got)
+	}
+	if got := escapeSQLString("a'b"); got != "a''b" {
+		t.Fatalf("escapeSQLString mismatch: %q", got)
+	}
+}

logger_more_test.go

@@ -0,0 +1,35 @@
+package norm
+
+import (
+	"expvar"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestExpvarMetricsUpdatesVars(t *testing.T) {
+	m := ExpvarMetrics{}
+	m.QueryDuration(12*time.Millisecond, "select 1")
+	m.ConnectionCount(3, 4)
+	m.ErrorCount("timeout")
+	m.CircuitStateChanged("open")
+
+	if got := expvar.Get("norm_query_count").String(); got == "0" {
+		t.Fatalf("query count not updated")
+	}
+	if got := expvar.Get("norm_last_query_ms").String(); got != "12" {
+		t.Fatalf("last query ms mismatch: %s", got)
+	}
+	if got := expvar.Get("norm_connections_active").String(); got != "3" {
+		t.Fatalf("active connections mismatch: %s", got)
+	}
+	if got := expvar.Get("norm_connections_idle").String(); got != "4" {
+		t.Fatalf("idle connections mismatch: %s", got)
+	}
+	if got := expvar.Get("norm_circuit_state").String(); got != "\"open\"" {
+		t.Fatalf("circuit state mismatch: %s", got)
+	}
+	if got := expvar.Get("norm_error_count").String(); !strings.Contains(got, "timeout") {
+		t.Fatalf("error count mismatch: %s", got)
+	}
+}

metrics_expvar_test.go

@@ -0,0 +1,53 @@
+package norm
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5/pgxpool"
+)
+
+func TestMakeLogFieldsAndPoolHelpers(t *testing.T) {
+	pool := &pgxpool.Pool{}
+	readPool := &pgxpool.Pool{}
+	kn := &KintsNorm{
+		pool:     pool,
+		readPool: readPool,
+		logContextFields: func(context.Context) []Field {
+			return []Field{{Key: "req_id", Value: "r1"}}
+		},
+	}
+
+	fields := kn.makeLogFields(context.Background(), "SELECT $1", []any{"x"})
+	if len(fields) != 4 {
+		t.Fatalf("unexpected field count: %d", len(fields))
+	}
+	if fields[0].Key != "req_id" || fields[1].Key != "sql" || fields[2].Key != "args" || fields[3].Key != "stmt" {
+		t.Fatalf("unexpected fields: %#v", fields)
+	}
+
+	kn.maskParams = true
+	fields = kn.makeLogFields(context.Background(), "SELECT $1", []any{"x"})
+	if len(fields) != 3 || fields[2].Value != "[masked]" {
+		t.Fatalf("masked fields mismatch: %#v", fields)
+	}
+
+	if kn.Pool() != pool {
+		t.Fatalf("pool accessor mismatch")
+	}
+	if kn.ReadPool() != readPool {
+		t.Fatalf("read pool accessor mismatch")
+	}
+	if qb := kn.QueryRead(); qb == nil || qb.exec == nil {
+		t.Fatalf("query read builder not initialized")
+	}
+
+	kn2 := &KintsNorm{}
+	if err := kn2.Close(); err != nil {
+		t.Fatalf("close nil pools: %v", err)
+	}
+	if defaultIfZeroDuration(0, time.Second) != time.Second {
+		t.Fatalf("helper regression")
+	}
+}