fix(core): resolve critical correctness defects in pools, metrics, AST, errors, keywords

Addresses 6 issues from the 2026-04-16 architect review:

C1+C2: Pool cleanup leaks in pkg/sql/ast/pool.go
- PutSelectStatement now releases GroupBy, Having, Qualify, StartWith, ConnectBy,
  Joins, Windows, PrewhereClause, Sample, ArrayJoin, Pivot, Unpivot,
  MatchRecognize, Top, DistinctOnColumns, From, Limit/Offset/Fetch/For, With.CTEs
- PutUpdateStatement, PutInsertStatement, PutDeleteStatement similarly audited
  and fixed
- PutExpression: raised MaxWorkQueueSize to 100k and added recursive overflow
  drain guarded by MaxCleanupDepth
- Added PoolLeakCount/ResetPoolLeakCount for observability
- New pool_leak_test.go proves stable heap across 1000 complex SELECTs and
  2000-element IN-lists

C3: Metrics errorsByType memory DoS in pkg/metrics/metrics.go
- Replaced err.Error()-keyed map with per-ErrorCode atomic.Int64 buckets
- GetStats now allocates 4/op regardless of error cardinality

C6: AST Children() coverage in pkg/sql/ast/
- Fixed 14 node types that silently truncated visitor traversal
- New children_coverage_test.go uses reflection to assert completeness

H6: errors.Error immutability in pkg/errors/errors.go
- WithContext/WithHint/WithCause shallow-copy receiver

H11: Keyword conflict detection in pkg/sql/keywords/
- New Conflicts()/ResetConflicts() API
- Resolved 11 silent conflicts across dialects

go test -race ./... passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 2 people

pkg/errors/errors.go

@@ -341,7 +341,7 @@ func NewError(code ErrorCode, message string, location models.Location) *Error {
 	}
 }
 
-// WithContext adds SQL context to the error.
+// WithContext returns a copy of the error with SQL context attached.
 //
 // Attaches SQL source code context with highlighting information for
 // visual error display. The context shows surrounding lines and highlights
@@ -351,7 +351,9 @@ func NewError(code ErrorCode, message string, location models.Location) *Error {
 //   - sql: Original SQL source code
 //   - highlightLen: Number of characters to highlight (starting at error column)
 //
-// Returns the same Error instance with context added (for method chaining).
+// Returns a new Error instance with context added; the receiver is not
+// modified. Callers MUST use the return value (e.g. `err = err.WithContext(...)`)
+// for the change to take effect.
 //
 // Example:
 //
@@ -363,27 +365,32 @@ func NewError(code ErrorCode, message string, location models.Location) *Error {
 //	1 | SELECT * FORM users
 //	           ^^^^
 //
-// Note: WithContext modifies the error in-place and returns it for chaining.
+// Immutability: WithContext is non-mutating — it returns a shallow copy
+// with the new field set. This makes *Error safe to share across goroutines
+// and call sites without observer effects.
 func (e *Error) WithContext(sql string, highlightLen int) *Error {
-	e.Context = &ErrorContext{
+	cp := *e
+	cp.Context = &ErrorContext{
 		SQL:          sql,
 		StartLine:    e.Location.Line,
 		EndLine:      e.Location.Line,
 		HighlightCol: e.Location.Column,
 		HighlightLen: highlightLen,
 	}
-	return e
+	return &cp
 }
 
-// WithHint adds a suggestion hint to the error.
+// WithHint returns a copy of the error with a suggestion hint attached.
 //
 // Attaches a helpful suggestion for fixing the error. Hints are generated
 // automatically by builder functions or can be added manually.
 //
 // Parameters:
 //   - hint: Suggestion text (e.g., "Did you mean 'FROM' instead of 'FORM'?")
 //
-// Returns the same Error instance with hint added (for method chaining).
+// Returns a new Error instance with hint added; the receiver is not modified.
+// Callers MUST use the return value (e.g. `err = err.WithHint(...)`) for the
+// change to take effect.
 //
 // Example:
 //
@@ -395,21 +402,26 @@ func (e *Error) WithContext(sql string, highlightLen int) *Error {
 //	err := errors.ExpectedTokenError("FROM", "FORM", location, sql)
 //	// Automatically includes: "Did you mean 'FROM' instead of 'FORM'?"
 //
-// Note: WithHint modifies the error in-place and returns it for chaining.
+// Immutability: WithHint is non-mutating — it returns a shallow copy with
+// the Hint field set. This makes *Error safe to share across goroutines
+// and call sites without observer effects.
 func (e *Error) WithHint(hint string) *Error {
-	e.Hint = hint
-	return e
+	cp := *e
+	cp.Hint = hint
+	return &cp
 }
 
-// WithCause adds an underlying cause error.
+// WithCause returns a copy of the error with an underlying cause attached.
 //
 // Wraps another error as the cause of this error, enabling error chaining
 // and unwrapping with errors.Is and errors.As.
 //
 // Parameters:
 //   - cause: The underlying error that caused this error
 //
-// Returns the same Error instance with cause added (for method chaining).
+// Returns a new Error instance with cause added; the receiver is not
+// modified. Callers MUST use the return value (e.g. `err = err.WithCause(...)`)
+// for the change to take effect.
 //
 // Example:
 //
@@ -425,10 +437,13 @@ func (e *Error) WithHint(hint string) *Error {
 //	    // Handle file not found
 //	}
 //
-// Note: WithCause modifies the error in-place and returns it for chaining.
+// Immutability: WithCause is non-mutating — it returns a shallow copy with
+// the Cause field set. This makes *Error safe to share across goroutines
+// and call sites without observer effects.
 func (e *Error) WithCause(cause error) *Error {
-	e.Cause = cause
-	return e
+	cp := *e
+	cp.Cause = cause
+	return &cp
 }
 
 // IsCode checks if an error has a specific error code.

pkg/errors/errors_test.go

@@ -84,7 +84,7 @@ func TestError_WithContext(t *testing.T) {
 	location := models.Location{Line: 1, Column: 10}
 
 	err := NewError(ErrCodeExpectedToken, "expected FROM, got FORM", location)
-	err.WithContext(sql, 4) // Highlight "FORM" (4 characters)
+	err = err.WithContext(sql, 4) // Highlight "FORM" (4 characters)
 
 	output := err.Error()
 
@@ -106,7 +106,7 @@ func TestError_WithContext(t *testing.T) {
 
 func TestError_WithHint(t *testing.T) {
 	err := NewError(ErrCodeUnexpectedToken, "unexpected token", models.Location{Line: 1, Column: 5})
-	err.WithHint("This is a helpful hint")
+	err = err.WithHint("This is a helpful hint")
 
 	if err.Hint != "This is a helpful hint" {
 		t.Errorf("WithHint() failed to set hint, got: %s", err.Hint)
@@ -197,7 +197,7 @@ WHERE age > 18`,
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			err := NewError(ErrCodeUnexpectedToken, "test", tt.location)
-			err.WithContext(tt.sql, 1)
+			err = err.WithContext(tt.sql, 1)
 
 			output := err.formatContext()
 
@@ -215,10 +215,127 @@ WHERE age > 18`,
 func TestError_Unwrap(t *testing.T) {
 	causeErr := NewError(ErrCodeInvalidSyntax, "cause error", models.Location{})
 	err := NewError(ErrCodeUnexpectedToken, "wrapper error", models.Location{})
-	err.WithCause(causeErr)
+	err = err.WithCause(causeErr)
 
 	unwrapped := err.Unwrap()
 	if unwrapped != causeErr {
 		t.Errorf("Unwrap() = %v, want %v", unwrapped, causeErr)
 	}
 }
+
+// TestError_WithContext_Immutable verifies WithContext does not mutate the
+// receiver: the original *Error is unchanged, and the returned *Error carries
+// the new Context. Guards against H6 regressions (observer effects across
+// call sites that share a *Error pointer).
+func TestError_WithContext_Immutable(t *testing.T) {
+	orig := NewError(ErrCodeUnexpectedToken, "msg", models.Location{Line: 1, Column: 5})
+	if orig.Context != nil {
+		t.Fatalf("precondition: fresh error should have nil Context, got %+v", orig.Context)
+	}
+
+	withCtx := orig.WithContext("SELECT * FORM users", 4)
+
+	if orig.Context != nil {
+		t.Fatalf("WithContext mutated the receiver: orig.Context = %+v", orig.Context)
+	}
+	if withCtx.Context == nil {
+		t.Fatalf("WithContext returned error without Context set")
+	}
+	if withCtx.Context.SQL != "SELECT * FORM users" {
+		t.Errorf("returned Context.SQL = %q, want %q", withCtx.Context.SQL, "SELECT * FORM users")
+	}
+	if withCtx.Context.HighlightLen != 4 {
+		t.Errorf("returned Context.HighlightLen = %d, want 4", withCtx.Context.HighlightLen)
+	}
+	if orig == withCtx {
+		t.Errorf("WithContext returned the same pointer; expected a copy")
+	}
+}
+
+// TestError_WithHint_Immutable verifies WithHint does not mutate the receiver.
+func TestError_WithHint_Immutable(t *testing.T) {
+	orig := NewError(ErrCodeUnexpectedToken, "msg", models.Location{Line: 1, Column: 5})
+	if orig.Hint != "" {
+		t.Fatalf("precondition: fresh error should have empty Hint, got %q", orig.Hint)
+	}
+
+	withHint := orig.WithHint("new hint")
+
+	if orig.Hint == "new hint" {
+		t.Fatal("WithHint mutated the receiver")
+	}
+	if orig.Hint != "" {
+		t.Fatalf("receiver Hint unexpectedly changed: %q", orig.Hint)
+	}
+	if withHint.Hint != "new hint" {
+		t.Fatalf("returned Error missing hint, got %q", withHint.Hint)
+	}
+	if orig == withHint {
+		t.Errorf("WithHint returned the same pointer; expected a copy")
+	}
+}
+
+// TestError_WithCause_Immutable verifies WithCause does not mutate the receiver.
+func TestError_WithCause_Immutable(t *testing.T) {
+	cause := NewError(ErrCodeInvalidSyntax, "root", models.Location{})
+	orig := NewError(ErrCodeUnexpectedToken, "wrapper", models.Location{})
+	if orig.Cause != nil {
+		t.Fatalf("precondition: fresh error should have nil Cause, got %v", orig.Cause)
+	}
+
+	withCause := orig.WithCause(cause)
+
+	if orig.Cause != nil {
+		t.Fatalf("WithCause mutated the receiver: orig.Cause = %v", orig.Cause)
+	}
+	if withCause.Cause != cause {
+		t.Fatalf("returned Error missing cause; got %v, want %v", withCause.Cause, cause)
+	}
+	if orig == withCause {
+		t.Errorf("WithCause returned the same pointer; expected a copy")
+	}
+}
+
+// TestError_WithX_SharedReceiver_NoObserverEffects simulates the production
+// bug: two call sites holding the same *Error pointer. Before the fix, one
+// call site's WithHint would be visible to the other. After the fix, each
+// caller gets an independent copy.
+func TestError_WithX_SharedReceiver_NoObserverEffects(t *testing.T) {
+	shared := NewError(ErrCodeUnexpectedToken, "msg", models.Location{Line: 1, Column: 1})
+
+	a := shared.WithHint("hint from A")
+	b := shared.WithHint("hint from B")
+
+	if shared.Hint != "" {
+		t.Fatalf("shared receiver was mutated: shared.Hint = %q", shared.Hint)
+	}
+	if a.Hint != "hint from A" {
+		t.Errorf("a.Hint = %q, want %q", a.Hint, "hint from A")
+	}
+	if b.Hint != "hint from B" {
+		t.Errorf("b.Hint = %q, want %q", b.Hint, "hint from B")
+	}
+	if a == b {
+		t.Errorf("both call sites got the same pointer; expected independent copies")
+	}
+}
+
+// TestError_WithX_Chaining verifies the `err = err.WithA(...).WithB(...)`
+// fluent pattern still accumulates all fields on the final returned error.
+func TestError_WithX_Chaining(t *testing.T) {
+	cause := NewError(ErrCodeInvalidSyntax, "root", models.Location{})
+	err := NewError(ErrCodeUnexpectedToken, "msg", models.Location{Line: 2, Column: 3}).
+		WithContext("SELECT 1", 1).
+		WithHint("a hint").
+		WithCause(cause)
+
+	if err.Context == nil || err.Context.SQL != "SELECT 1" {
+		t.Errorf("chained WithContext not applied: %+v", err.Context)
+	}
+	if err.Hint != "a hint" {
+		t.Errorf("chained WithHint not applied: %q", err.Hint)
+	}
+	if err.Cause != cause {
+		t.Errorf("chained WithCause not applied: %v", err.Cause)
+	}
+}

pkg/errors/example_test.go

@@ -144,9 +144,9 @@ func Example_customHints() {
 		errors.ErrCodeIncompleteStatement,
 		"incomplete WHERE clause",
 		location,
-	)
-	err.WithContext(sql, 5)
-	err.WithHint("Add a condition after WHERE, e.g., WHERE age > 18")
+	).
+		WithContext(sql, 5).
+		WithHint("Add a condition after WHERE, e.g., WHERE age > 18")
 
 	// Error now includes custom context and hint
 	_ = err

pkg/errors/examples_test.go

@@ -210,9 +210,9 @@ func Example_customHintsEnhanced() {
 		errors.ErrCodeInvalidSyntax,
 		"type mismatch in comparison",
 		location,
-	)
-	err.WithContext(sql, 4) // Highlight '18'
-	err.WithHint("Age comparisons should use numeric values without quotes. Change '18' to 18")
+	).
+		WithContext(sql, 4). // Highlight '18'
+		WithHint("Age comparisons should use numeric values without quotes. Change '18' to 18")
 
 	fmt.Println("Custom Hint Example:")
 	fmt.Println(err.Error())

pkg/errors/formatter_test.go

@@ -553,8 +553,8 @@ WHERE 年龄 > 18`,
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := NewError(ErrCodeUnexpectedToken, "test error", tt.location)
-			err.WithContext(tt.sql, 1)
+			err := NewError(ErrCodeUnexpectedToken, "test error", tt.location).
+				WithContext(tt.sql, 1)
 
 			got := err.Error()