Răsfoiți Sursa

resolve catalog predicates before a locked scan

A regclass/regtype cast or catalog function evaluated inside a statement's row
scan called buildCatalogSnapshot, which reads the session catalog
(GetSchema/ListTables/GetIndex) and so re-enters the session lock. That lock is
already held by the scan: Session.UpdateFunc/Delete hold it for an implicit
transaction's update, and every statement inside BEGIN holds it for its scan,
so the statement deadlocked forever and leaked the table gate to every other
session.

Root cause: catalog evaluation was not re-entrant with respect to the session
lock. Fix it the same way the array type scope is handled - resolve before the
scan:

- Execute primes the statement's catalog snapshot before dispatch, when the
  statement can evaluate a regclass/regtype cast, a snapshot-backed catalog
  function (format_type, pg_table_is_visible, pg_get_indexdef,
  pg_get_constraintdef, has_table_privilege, to_regclass, to_regtype), or a
  subquery containing either. The walker covers SELECT (columns, FROM, joins,
  derived tables, WHERE, GROUP BY, HAVING, ORDER BY, LIMIT/OFFSET, CTEs,
  compounds), UPDATE (SET, FROM, WHERE, RETURNING), DELETE (WHERE, RETURNING)
  and INSERT (VALUES, ON CONFLICT, SELECT, RETURNING).
- executeSelect primes its own body before scanning, so a view or CTE body that
  the outer statement never saw is primed too.
- catalogRelationFor resolves the catalog name before asking the session, and
  catalogTableExists answers shadowing from the pre-built snapshot while a scan
  holds the session lock; snapshotTypeResolver skips virtual catalog relations
  instead of reading a schema for them.

Defense in depth: row filters (SELECT pushdown, UPDATE/DELETE predicates) mark
that a scan holds the session lock. A catalog read that still finds no snapshot
returns 0A000 instead of deadlocking, and a subquery that would need the durable
schema from inside the lock (a correlated predicate) reports 0A000 instead of
hanging. SET expressions are not marked: the storage session releases its lock
around the update callback, which is why correlated scalar subqueries in SET
keep working.

Regression tests run every shape under a bounded timeout and probe a second
session for reads and writes afterwards, assert 42P01, assert no partial write,
and cover UPDATE/DELETE (autocommit and explicit transaction), IN subqueries,
catalog joins, SET/RETURNING catalog expressions, correlated catalog subqueries,
and the correlated base-table predicate that now errors instead of deadlocking.
The new pgserver test drives the same shapes over the wire and checks a second
connection is usable.
Danilo Fragoso 1 zi în urmă
părinte
comite
f1cf801ded

+ 39 - 3
pkg/executor/executor.go

@@ -42,8 +42,11 @@ type Executor struct {
 
 	// pgCatalog caches the virtual PostgreSQL catalog snapshot for the current
 	// statement. It is discarded by Execute so each statement observes DDL
-	// committed (or staged) since the previous one.
-	pgCatalog *catalogSnapshot
+	// committed (or staged) since the previous one. A statement that evaluates
+	// catalog-backed expressions primes it before scanning; lockedScanDepth then
+	// reports that the session lock is already held.
+	pgCatalog       *catalogSnapshot
+	lockedScanDepth int
 
 	// searchPath is the session search path used by current_schema() and SHOW
 	// search_path. Only public, pg_catalog, and information_schema are
@@ -297,6 +300,14 @@ func (e *Executor) Execute(stmt parser.Statement) (*Result, error) {
 		e.pgCatalog = nil
 	}()
 
+	// Catalog-backed expressions resolve names through a snapshot that reads the
+	// session catalog. That read must happen before a scan takes the session lock,
+	// so a statement using regclass/regtype or a catalog function primes the
+	// snapshot here.
+	if err := e.primeCatalogSnapshot(stmt); err != nil {
+		return nil, err
+	}
+
 	// PRAGMA doesn't need analysis. SQLite catalog-introspection pragmas
 	// (index_list, index_info, table_xinfo) are answered by the catalog agent
 	// first; everything else falls through to the built-in handler.
@@ -501,6 +512,23 @@ func isCountStarSingleTable(stmt *parser.SelectStmt) bool {
 
 // executeSelect executes a SELECT statement (or compound SELECT).
 func (e *Executor) executeSelect(stmt *parser.SelectStmt) (*Result, error) {
+	// A subquery evaluated inside a scan that already holds the session lock (a
+	// correlated predicate) cannot read the durable schema: doing so re-enters
+	// the session lock and deadlocks, leaking the table gate to every other
+	// session. Only a SELECT that resolves entirely from virtual catalog
+	// relations, materialized CTEs, or no FROM at all may run in that state.
+	if e.lockedScanDepth > 0 && !e.fromResolvesToCatalogRelations(stmt.From) {
+		return nil, pgerror.Unsupported("a correlated subquery cannot be evaluated inside a locked table scan; its predicate must not read the row being written")
+	}
+	// This SELECT may be a view or CTE body that the statement-level prime never
+	// saw, and its WHERE is pushed into the scan below, so resolve the catalog
+	// snapshot before the scan starts.
+	if e.pgCatalog == nil && catalogSnapshotSelectNeeded(stmt) {
+		if _, err := e.catalogSnapshotForQuery(); err != nil {
+			return nil, err
+		}
+	}
+
 	// The join scope is pushed below the snapshot so a real table's unqualified
 	// column keeps precedence; it answers for right-side derived tables and
 	// CTEs that the snapshot cannot read without materializing them again.
@@ -761,6 +789,8 @@ func (e *Executor) executeSelect(stmt *parser.SelectStmt) (*Result, error) {
 		if canFilterInScan {
 			appliedInScan = true
 			filter = func(row storage.Row) bool {
+				e.enterLockedScan()
+				defer e.exitLockedScan()
 				val, ferr := e.evalExpr(effectiveWhere, row)
 				if ferr != nil {
 					filterErr = ferr
@@ -3687,6 +3717,8 @@ func (e *Executor) executeUpdate(stmt *parser.UpdateStmt) (*Result, error) {
 	var filter func(storage.Row) bool
 	if stmt.Where != nil {
 		filter = func(row storage.Row) bool {
+			e.enterLockedScan()
+			defer e.exitLockedScan()
 			val, err := e.evalExpr(stmt.Where, row)
 			if err != nil {
 				filterErr = err
@@ -3698,7 +3730,9 @@ func (e *Executor) executeUpdate(stmt *parser.UpdateStmt) (*Result, error) {
 
 	// Use UpdateFunc to evaluate expressions per-row (supports self-referencing like balance = balance + 100).
 	// Generated columns are recomputed from the post-assignment row and written
-	// back so STORED values stay consistent.
+	// back so STORED values stay consistent. The storage session releases its
+	// lock around this callback (runUpdateFn), so SET expressions may read the
+	// session (a scalar subquery) while the surrounding scan holds nothing.
 	updateFn := func(row storage.Row) (storage.Row, error) {
 		updates := make(storage.Row)
 		merged := make(storage.Row, len(row))
@@ -4086,6 +4120,8 @@ func (e *Executor) executeDelete(stmt *parser.DeleteStmt) (*Result, error) {
 	var filter func(storage.Row) bool
 	if stmt.Where != nil {
 		filter = func(row storage.Row) bool {
+			e.enterLockedScan()
+			defer e.exitLockedScan()
 			val, err := e.evalExpr(stmt.Where, row)
 			if err != nil {
 				filterErr = err

+ 286 - 4
pkg/executor/pg_catalog.go

@@ -280,11 +280,14 @@ func (e *Executor) catalogRelationFor(ref parser.TableRef) (catalogRelationEntry
 		entry, ok := pgCatalogRelationIndex[informationSchemaName+"."+name]
 		return entry, ok
 	case "":
-		if e.session.TableExists(ref.Name) {
+		// Resolve the catalog name first: an ordinary table name then needs no
+		// session lookup, and a locked scan only asks the session whether a
+		// catalog-named relation is shadowed by a durable table.
+		entry, ok := pgCatalogRelationIndex[pgCatalogSchemaName+"."+name]
+		if !ok || e.catalogTableExists(ref.Name) {
 			return catalogRelationEntry{}, false
 		}
-		entry, ok := pgCatalogRelationIndex[pgCatalogSchemaName+"."+name]
-		return entry, ok
+		return entry, true
 	default:
 		// The emulated schemas are distinct: public.<name> names a durable
 		// relation, never pg_catalog.<name>, so a qualified miss falls through to
@@ -572,6 +575,14 @@ func (e *Executor) catalogSnapshotForQuery() (*catalogSnapshot, error) {
 	if e.pgCatalog != nil {
 		return e.pgCatalog, nil
 	}
+	if e.lockedScanDepth > 0 {
+		// Building a snapshot reads the session catalog, which takes the session
+		// lock; that lock is already held by the scan this predicate runs inside,
+		// so continuing would deadlock the connection and leak the table lock.
+		// Statements that can reach this point prime the snapshot before scanning
+		// (see primeCatalogSnapshot), so this is a guard, not a policy.
+		return nil, pgerror.Unsupported("catalog relations cannot be resolved inside a locked table scan")
+	}
 	snapshot, err := buildCatalogSnapshot(e)
 	if err != nil {
 		return nil, err
@@ -580,6 +591,236 @@ func (e *Executor) catalogSnapshotForQuery() (*catalogSnapshot, error) {
 	return snapshot, nil
 }
 
+// catalogSnapshotFunctionNames lists the catalog functions whose value is
+// resolved through the statement's virtual catalog snapshot.
+var catalogSnapshotFunctionNames = map[string]bool{
+	"FORMAT_TYPE":          true,
+	"PG_TABLE_IS_VISIBLE":  true,
+	"PG_GET_INDEXDEF":      true,
+	"PG_GET_CONSTRAINTDEF": true,
+	"HAS_TABLE_PRIVILEGE":  true,
+	"TO_REGCLASS":          true,
+	"TO_REGTYPE":           true,
+}
+
+// catalogSnapshotExprNeeded reports whether evaluating expr reads the virtual
+// catalog at value time: a regclass/regtype cast, a catalog function that
+// resolves names through the snapshot, or a subquery containing either.
+// AST fields are often typed nil pointers inside an interface, so every pointer
+// case checks for nil before reading it.
+func catalogSnapshotExprNeeded(expr parser.Expr) bool {
+	switch n := expr.(type) {
+	case nil, *parser.ColumnRef, *parser.LiteralExpr:
+		return false
+	case *parser.CastExpr:
+		if n == nil {
+			return false
+		}
+		switch strings.ToUpper(strings.TrimSpace(n.Type.SQLTypeName())) {
+		case "REGCLASS", "REGTYPE":
+			return true
+		}
+		return catalogSnapshotExprNeeded(n.Expr)
+	case *parser.FunctionCall:
+		if n == nil {
+			return false
+		}
+		if catalogSnapshotFunctionNames[strings.ToUpper(n.Name)] {
+			return true
+		}
+		for _, arg := range n.Args {
+			if catalogSnapshotExprNeeded(arg) {
+				return true
+			}
+		}
+		return false
+	case *parser.BinaryExpr:
+		return n != nil && (catalogSnapshotExprNeeded(n.Left) || catalogSnapshotExprNeeded(n.Right))
+	case *parser.UnaryExpr:
+		return n != nil && catalogSnapshotExprNeeded(n.Operand)
+	case *parser.BetweenExpr:
+		return n != nil && (catalogSnapshotExprNeeded(n.Left) || catalogSnapshotExprNeeded(n.Low) || catalogSnapshotExprNeeded(n.High))
+	case *parser.LikeExpr:
+		return n != nil && (catalogSnapshotExprNeeded(n.Left) || catalogSnapshotExprNeeded(n.Pattern) || catalogSnapshotExprNeeded(n.Escape))
+	case *parser.IsNullExpr:
+		return n != nil && catalogSnapshotExprNeeded(n.Left)
+	case *parser.IsDistinctExpr:
+		return n != nil && (catalogSnapshotExprNeeded(n.Left) || catalogSnapshotExprNeeded(n.Right))
+	case *parser.CaseExpr:
+		if n == nil {
+			return false
+		}
+		if catalogSnapshotExprNeeded(n.Operand) || catalogSnapshotExprNeeded(n.Else) {
+			return true
+		}
+		for _, when := range n.Whens {
+			if catalogSnapshotExprNeeded(when.Condition) || catalogSnapshotExprNeeded(when.Result) {
+				return true
+			}
+		}
+		return false
+	case *parser.ArrayExpr:
+		if n == nil {
+			return false
+		}
+		for _, element := range n.Elements {
+			if catalogSnapshotExprNeeded(element) {
+				return true
+			}
+		}
+		return false
+	case *parser.SubscriptExpr:
+		return n != nil && (catalogSnapshotExprNeeded(n.Expr) || catalogSnapshotExprNeeded(n.Index))
+	case *parser.AnyAllExpr:
+		return n != nil && (catalogSnapshotExprNeeded(n.Left) || catalogSnapshotExprNeeded(n.Array))
+	case *parser.InExpr:
+		if n == nil {
+			return false
+		}
+		if catalogSnapshotExprNeeded(n.Left) || catalogSnapshotSelectNeeded(n.Subquery) {
+			return true
+		}
+		for _, value := range n.Values {
+			if catalogSnapshotExprNeeded(value) {
+				return true
+			}
+		}
+		return false
+	case *parser.SubqueryExpr:
+		return n != nil && catalogSnapshotSelectNeeded(n.Query)
+	case *parser.ExistsExpr:
+		return n != nil && catalogSnapshotSelectNeeded(n.Subquery)
+	case *parser.ParenExpr:
+		return n != nil && catalogSnapshotExprNeeded(n.Expr)
+	default:
+		return false
+	}
+}
+
+// catalogSnapshotSelectNeeded reports whether a SELECT evaluates a catalog-backed
+// expression in any of its clauses or nested relations.
+func catalogSnapshotSelectNeeded(sel *parser.SelectStmt) bool {
+	if sel == nil {
+		return false
+	}
+	for _, cte := range sel.With {
+		if catalogSnapshotSelectNeeded(cte.Query) {
+			return true
+		}
+	}
+	if sel.Compound != nil && (catalogSnapshotSelectNeeded(sel.Compound.Left) || catalogSnapshotSelectNeeded(sel.Compound.Right)) {
+		return true
+	}
+	for _, column := range sel.Columns {
+		if catalogSnapshotExprNeeded(column.Expr) {
+			return true
+		}
+	}
+	for i := range sel.From {
+		if catalogSnapshotTableRefNeeded(&sel.From[i]) {
+			return true
+		}
+	}
+	for _, expr := range sel.GroupBy {
+		if catalogSnapshotExprNeeded(expr) {
+			return true
+		}
+	}
+	for _, item := range sel.OrderBy {
+		if catalogSnapshotExprNeeded(item.Expr) {
+			return true
+		}
+	}
+	return catalogSnapshotExprNeeded(sel.Where) ||
+		catalogSnapshotExprNeeded(sel.Having) ||
+		catalogSnapshotExprNeeded(sel.Limit) ||
+		catalogSnapshotExprNeeded(sel.Offset)
+}
+
+// catalogSnapshotTableRefNeeded reports whether a FROM entry evaluates a
+// catalog-backed expression in a derived table, table function, or join.
+func catalogSnapshotTableRefNeeded(ref *parser.TableRef) bool {
+	if ref == nil {
+		return false
+	}
+	if catalogSnapshotSelectNeeded(ref.Subquery) || catalogSnapshotExprNeeded(ref.Function) {
+		return true
+	}
+	if ref.Join != nil {
+		return catalogSnapshotTableRefNeeded(ref.Join.Table) || catalogSnapshotExprNeeded(ref.Join.Condition)
+	}
+	return false
+}
+
+// catalogSnapshotStatementNeeded reports whether the statement evaluates a
+// catalog-backed expression anywhere it can run during the statement.
+func catalogSnapshotStatementNeeded(stmt parser.Statement) bool {
+	switch s := stmt.(type) {
+	case *parser.SelectStmt:
+		return catalogSnapshotSelectNeeded(s)
+	case *parser.UpdateStmt:
+		for i := range s.From {
+			if catalogSnapshotTableRefNeeded(&s.From[i]) {
+				return true
+			}
+		}
+		for _, assignment := range s.Set {
+			if catalogSnapshotExprNeeded(assignment.Value) {
+				return true
+			}
+		}
+		return catalogSnapshotExprNeeded(s.Where) || catalogSnapshotReturningNeeded(s.Returning)
+	case *parser.DeleteStmt:
+		return catalogSnapshotExprNeeded(s.Where) || catalogSnapshotReturningNeeded(s.Returning)
+	case *parser.InsertStmt:
+		for _, row := range s.Values {
+			for _, value := range row {
+				if catalogSnapshotExprNeeded(value) {
+					return true
+				}
+			}
+		}
+		for _, assignment := range s.ConflictUpdate {
+			if catalogSnapshotExprNeeded(assignment.Value) {
+				return true
+			}
+		}
+		return catalogSnapshotSelectNeeded(s.Select) || catalogSnapshotReturningNeeded(s.Returning)
+	default:
+		return false
+	}
+}
+
+func catalogSnapshotReturningNeeded(columns []parser.SelectColumn) bool {
+	for _, column := range columns {
+		if catalogSnapshotExprNeeded(column.Expr) {
+			return true
+		}
+	}
+	return false
+}
+
+// primeCatalogSnapshot builds the statement's virtual catalog snapshot before a
+// scan can take the session lock. A regclass/regtype cast or catalog function
+// evaluated inside a locked scan would otherwise call back into the session while
+// the scan holds its lock, which deadlocks the connection and leaks the table
+// lock to every other session. Statements whose value-time expressions cannot
+// reach the catalog skip the build entirely.
+func (e *Executor) primeCatalogSnapshot(stmt parser.Statement) error {
+	if e.pgCatalog != nil || !catalogSnapshotStatementNeeded(stmt) {
+		return nil
+	}
+	_, err := e.catalogSnapshotForQuery()
+	return err
+}
+
+// enterLockedScan marks the start of row-filter or update evaluation that runs
+// inside a session scan, where the session lock is already held.
+func (e *Executor) enterLockedScan() { e.lockedScanDepth++ }
+
+// exitLockedScan marks the end of one such evaluation.
+func (e *Executor) exitLockedScan() { e.lockedScanDepth-- }
+
 // visibleCatalogColumns drops the engine's hidden synthetic rowid column, which
 // is not a user-visible PostgreSQL attribute.
 func visibleCatalogColumns(schema *storage.Schema) []storage.Column {
@@ -1815,12 +2056,53 @@ func (s *catalogSnapshot) informationSchemaSchemataRows() []storage.Row {
 // not shadowed by a durable table or a CTE.
 func (e *Executor) catalogSchemaByName(name string) (*storage.Schema, bool) {
 	entry, ok := pgCatalogRelationIndex[pgCatalogSchemaName+"."+strings.ToLower(name)]
-	if !ok || e.session.TableExists(name) || e.cteTableExists(name) {
+	if !ok || e.catalogTableExists(name) || e.cteTableExists(name) {
 		return nil, false
 	}
 	return entry.schema, true
 }
 
+// fromResolvesToCatalogRelations reports whether every FROM entry is a virtual
+// catalog relation or a materialized CTE, so the statement can be evaluated
+// without reading the durable schema. A statement with no FROM needs no schema
+// either.
+func (e *Executor) fromResolvesToCatalogRelations(from []parser.TableRef) bool {
+	for i := range from {
+		ref := &from[i]
+		if e.cteTableExists(ref.Name) {
+			continue
+		}
+		if _, ok := e.catalogRelationFor(*ref); !ok {
+			return false
+		}
+		for join := ref.Join; join != nil && join.Table != nil; join = join.Table.Join {
+			if e.cteTableExists(join.Table.Name) {
+				continue
+			}
+			if _, ok := e.catalogRelationFor(*join.Table); !ok {
+				return false
+			}
+		}
+	}
+	return true
+}
+
+// catalogTableExists reports whether a durable table shadows an unqualified
+// catalog relation name. A scan that already holds the session lock cannot ask
+// the session, so the statement's pre-built snapshot answers instead; without
+// one the virtual catalog relation is assumed, which makes the catalog path
+// report the locked-scan guard error instead of deadlocking.
+func (e *Executor) catalogTableExists(name string) bool {
+	if e.lockedScanDepth > 0 {
+		if e.pgCatalog == nil {
+			return false
+		}
+		_, ok := e.pgCatalog.tableByName[strings.ToLower(name)]
+		return ok
+	}
+	return e.session.TableExists(name)
+}
+
 func (s *catalogSnapshot) informationSchemaViewsRows() []storage.Row {
 	// A session view is listed here exactly when it is listed in pg_class with
 	// relkind 'v'. The engine does not retain the view's SQL text, so the

+ 280 - 0
pkg/executor/pg_catalog_lock_test.go

@@ -0,0 +1,280 @@
+package executor
+
+import (
+	"errors"
+	"fmt"
+	"runtime"
+	"testing"
+	"time"
+
+	"github.com/danfragoso/pizzasql-next/pkg/pgerror"
+	"github.com/danfragoso/pizzasql-next/pkg/storage"
+)
+
+// A catalog-backed predicate must be resolved before a statement takes the
+// session lock. Evaluating it inside the locked scan re-enters the session lock
+// (which is not re-entrant), so the statement hangs forever and every other
+// session blocks on the table the stuck statement never releases.
+//
+// These regressions therefore run every statement under a bounded timeout and
+// probe a second session afterwards.
+const catalogLockTimeout = 10 * time.Second
+
+type catalogLockOutcome struct {
+	result *Result
+	err    error
+}
+
+// runCatalogLockedStatement executes one statement and fails the test when it
+// does not finish within catalogLockTimeout.
+func runCatalogLockedStatement(t *testing.T, e *Executor, sql string) (*Result, error) {
+	t.Helper()
+	done := make(chan catalogLockOutcome, 1)
+	go func() {
+		result, err := execSQL(e, sql)
+		done <- catalogLockOutcome{result: result, err: err}
+	}()
+	select {
+	case out := <-done:
+		return out.result, out.err
+	case <-time.After(catalogLockTimeout):
+		buf := make([]byte, 1<<20)
+		n := runtime.Stack(buf, true)
+		t.Fatalf("%s did not finish within %s (deadlock)\n%s", sql, catalogLockTimeout, buf[:n])
+		return nil, nil
+	}
+}
+
+// probeSessionLiveness proves a second connection can still read and write: a
+// statement that deadlocks while holding the session lock leaks the table gate,
+// so a later session's SELECT or INSERT blocks forever.
+func probeSessionLiveness(t *testing.T, schema *storage.SchemaManager, table *storage.TableManager, tableName string) {
+	t.Helper()
+	observer := newExec(schema, table)
+	if _, err := runCatalogLockedStatement(t, observer, "SELECT count(*) FROM "+tableName); err != nil {
+		t.Fatalf("observer SELECT on %s failed: %v", tableName, err)
+	}
+	if _, err := runCatalogLockedStatement(t, observer, "INSERT INTO probe (id) VALUES (1)"); err != nil {
+		t.Fatalf("observer INSERT failed: %v", err)
+	}
+}
+
+// setupCatalogLockFixture builds the target table plus an unrelated table the
+// observer session writes to.
+func setupCatalogLockFixture(t *testing.T) (*Executor, *storage.SchemaManager, *storage.TableManager) {
+	t.Helper()
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)")
+	execMust(t, e, "INSERT INTO t VALUES (1, 'a'), (2, 'b')")
+	execMust(t, e, "CREATE TABLE other (id INTEGER PRIMARY KEY)")
+	execMust(t, e, "INSERT INTO other VALUES (1)")
+	execMust(t, e, "CREATE TABLE probe (id INTEGER PRIMARY KEY)")
+	return e, schema, table
+}
+
+func TestPGCatalogPredicateInLockedScanDoesNotDeadlock(t *testing.T) {
+	cases := []struct {
+		name      string
+		sql       string
+		wantError bool
+		wantRows  int
+		wantValue string
+	}{
+		{
+			name:      "update unknown regclass",
+			sql:       `UPDATE t SET v = 'z' WHERE v = 'a' AND 'missing_relation'::regclass IS NOT NULL`,
+			wantError: true,
+		},
+		{
+			name:     "update known regclass",
+			sql:      `UPDATE t SET v = 'z' WHERE v = 'a' AND 't'::regclass IS NOT NULL`,
+			wantRows: 1,
+		},
+		{
+			name:      "delete unknown regclass",
+			sql:       `DELETE FROM t WHERE v = 'a' AND 'missing_relation'::regclass IS NOT NULL`,
+			wantError: true,
+		},
+		{
+			name:     "update catalog function predicate",
+			sql:      `UPDATE t SET v = 'z' WHERE v = 'a' AND format_type(23, -1) = 'integer'`,
+			wantRows: 1,
+		},
+		{
+			name:     "update in catalog subquery",
+			sql:      `UPDATE t SET v = 'z' WHERE id IN (SELECT oid FROM pg_class)`,
+			wantRows: 0,
+		},
+		{
+			name:     "update in constant subquery",
+			sql:      `UPDATE t SET v = 'z' WHERE id IN (SELECT 1)`,
+			wantRows: 1,
+		},
+		{
+			name:     "update in base-table subquery",
+			sql:      `UPDATE t SET v = 'z' WHERE id IN (SELECT id FROM other)`,
+			wantRows: 1,
+		},
+		{
+			name:     "update returning regclass",
+			sql:      `UPDATE t SET v = 'z' WHERE id = 1 RETURNING id, 't'::regclass`,
+			wantRows: 1,
+		},
+		{
+			name:      "update set catalog function",
+			sql:       `UPDATE t SET v = 'fmt:' || format_type(23, -1) WHERE id = 1`,
+			wantValue: "fmt:integer",
+		},
+		{
+			name:     "update correlated catalog subquery",
+			sql:      `UPDATE t SET v = 'z' WHERE EXISTS (SELECT 1 FROM pg_class c WHERE c.relname = t.v)`,
+			wantRows: 0,
+		},
+		{
+			name:     "update catalog relation predicate",
+			sql:      `UPDATE t SET v = 'z' WHERE id IN (SELECT c.oid FROM pg_class c JOIN pg_attribute a ON a.attrelid = c.oid)`,
+			wantRows: 0,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			e, schema, table := setupCatalogLockFixture(t)
+			_, err := runCatalogLockedStatement(t, e, tc.sql)
+			if tc.wantError {
+				if !errors.Is(err, pgerror.ErrUndefinedTable) {
+					t.Fatalf("%s: err = %v, want an undefined_table error", tc.sql, err)
+				}
+			} else if err != nil {
+				t.Fatalf("%s: %v", tc.sql, err)
+			} else if tc.wantRows > 0 {
+				res, countErr := runCatalogLockedStatement(t, e, `SELECT count(*) FROM t WHERE v = 'z'`)
+				if countErr != nil {
+					t.Fatal(countErr)
+				}
+				if res.Rows[0][0] != int64(tc.wantRows) {
+					t.Fatalf("%s: updated rows = %v, want %d", tc.sql, res.Rows[0][0], tc.wantRows)
+				}
+			} else if tc.wantValue != "" {
+				res, valueErr := runCatalogLockedStatement(t, e, `SELECT v FROM t WHERE id = 1`)
+				if valueErr != nil {
+					t.Fatal(valueErr)
+				}
+				if fmt.Sprintf("%v", res.Rows[0][0]) != tc.wantValue {
+					t.Fatalf("%s: v = %v, want %q", tc.sql, res.Rows[0][0], tc.wantValue)
+				}
+			}
+			probeSessionLiveness(t, schema, table, "t")
+		})
+	}
+}
+
+// A predicate that fails part-way through the scan must not leave rows written
+// by the rows the scan already visited.
+func TestPGCatalogPredicateErrorLeavesNoPartialWrite(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	// No UNIQUE index: nothing else makes the statement atomic for it.
+	execMust(t, e, "CREATE TABLE plain (id INTEGER, v TEXT)")
+	execMust(t, e, "INSERT INTO plain VALUES (1, 'one'), (2, 'two'), (3, 'three')")
+	execMust(t, e, "CREATE TABLE probe (id INTEGER PRIMARY KEY)")
+
+	// The predicate succeeds for rows 1 and 3 and fails on row 2, so a
+	// non-atomic update would leave rows 1 and 3 changed.
+	_, err := runCatalogLockedStatement(t, e, `UPDATE plain SET v = 'changed' WHERE id <> 2 OR 'missing_relation'::regclass IS NOT NULL`)
+	if !errors.Is(err, pgerror.ErrUndefinedTable) {
+		t.Fatalf("update err = %v, want an undefined_table error", err)
+	}
+	res := execMust(t, e, `SELECT count(*) FROM plain WHERE v = 'changed'`)
+	if res.Rows[0][0] != int64(0) {
+		t.Fatalf("partial write: %v rows changed", res.Rows[0][0])
+	}
+
+	_, err = runCatalogLockedStatement(t, e, `DELETE FROM plain WHERE id <> 2 OR 'missing_relation'::regclass IS NOT NULL`)
+	if !errors.Is(err, pgerror.ErrUndefinedTable) {
+		t.Fatalf("delete err = %v, want an undefined_table error", err)
+	}
+	res = execMust(t, e, `SELECT count(*) FROM plain`)
+	if res.Rows[0][0] != int64(3) {
+		t.Fatalf("partial delete: %v rows left, want 3", res.Rows[0][0])
+	}
+	probeSessionLiveness(t, schema, table, "plain")
+}
+
+// The same deadlock class applies to any scan that runs inside an open
+// transaction, including a SELECT.
+func TestPGCatalogPredicateInExplicitTransactionDoesNotDeadlock(t *testing.T) {
+	e, schema, table := setupCatalogLockFixture(t)
+
+	if _, err := runCatalogLockedStatement(t, e, "BEGIN"); err != nil {
+		t.Fatalf("BEGIN: %v", err)
+	}
+	_, err := runCatalogLockedStatement(t, e, `SELECT * FROM t WHERE v = 'a' AND 'missing_relation'::regclass IS NOT NULL`)
+	if !errors.Is(err, pgerror.ErrUndefinedTable) {
+		t.Fatalf("select err = %v, want an undefined_table error", err)
+	}
+	_, err = runCatalogLockedStatement(t, e, `UPDATE t SET v = 'z' WHERE v = 'a' AND 'missing_relation'::regclass IS NOT NULL`)
+	if !errors.Is(err, pgerror.ErrUndefinedTable) {
+		t.Fatalf("update err = %v, want an undefined_table error", err)
+	}
+	// A catalog relation in a predicate still works inside the transaction.
+	res, err := runCatalogLockedStatement(t, e, `SELECT count(*) FROM t WHERE id IN (SELECT 1)`)
+	if err != nil {
+		t.Fatalf("catalog-free predicate: %v", err)
+	}
+	if res.Rows[0][0] != int64(1) {
+		t.Fatalf("count = %v, want 1", res.Rows[0][0])
+	}
+	if _, err := runCatalogLockedStatement(t, e, "ROLLBACK"); err != nil {
+		t.Fatalf("ROLLBACK: %v", err)
+	}
+
+	// The transaction held no lock after the rollback.
+	probeSessionLiveness(t, schema, table, "t")
+
+	// A catalog predicate that matches still updates inside a transaction.
+	if _, err := runCatalogLockedStatement(t, e, "BEGIN"); err != nil {
+		t.Fatalf("BEGIN: %v", err)
+	}
+	if _, err := runCatalogLockedStatement(t, e, `UPDATE t SET v = 'z' WHERE id = 2 AND 't'::regclass IS NOT NULL`); err != nil {
+		t.Fatalf("update in transaction: %v", err)
+	}
+	if _, err := runCatalogLockedStatement(t, e, "COMMIT"); err != nil {
+		t.Fatalf("COMMIT: %v", err)
+	}
+	res = execMust(t, e, `SELECT v FROM t WHERE id = 2`)
+	if fmt.Sprintf("%v", res.Rows[0][0]) != "z" {
+		t.Fatalf("committed value = %v, want z", res.Rows[0][0])
+	}
+}
+
+// A correlated subquery in a write predicate cannot read the durable schema while
+// the scan holds the session lock. It must report that (0A000) instead of
+// deadlocking, and the failed statement must leave no partial write and no lock.
+func TestPGCatalogCorrelatedPredicateErrorsInsteadOfDeadlocking(t *testing.T) {
+	e, schema, table := setupCatalogLockFixture(t)
+	_, err := runCatalogLockedStatement(t, e, `UPDATE t SET v = 'z' WHERE EXISTS (SELECT 1 FROM other o WHERE o.id = t.id)`)
+	if !errors.Is(err, pgerror.ErrFeatureNotSupported) {
+		t.Fatalf("err = %v, want a feature_not_supported error", err)
+	}
+	res := execMust(t, e, `SELECT count(*) FROM t WHERE v = 'z'`)
+	if res.Rows[0][0] != int64(0) {
+		t.Fatalf("partial write: %v rows changed", res.Rows[0][0])
+	}
+	probeSessionLiveness(t, schema, table, "t")
+}
+
+// A catalog-only correlated subquery resolves from the statement snapshot, so it
+// still works inside the locked scan.
+func TestPGCatalogCorrelatedSubqueryResolvesFromSnapshot(t *testing.T) {
+	e, schema, table := setupCatalogLockFixture(t)
+	if _, err := runCatalogLockedStatement(t, e, `UPDATE t SET v = 'pg_class' WHERE EXISTS (SELECT 1 FROM pg_class c WHERE c.relname = 'pg_class')`); err != nil {
+		t.Fatalf("catalog-only correlated subquery: %v", err)
+	}
+	res := execMust(t, e, `SELECT count(*) FROM t WHERE v = 'pg_class'`)
+	if res.Rows[0][0] != int64(2) {
+		t.Fatalf("updated rows = %v, want 2", res.Rows[0][0])
+	}
+	probeSessionLiveness(t, schema, table, "t")
+}

+ 6 - 0
pkg/executor/result_types.go

@@ -33,6 +33,12 @@ func (e *Executor) snapshotTypeResolver(refs []parser.TableRef) columnTypeResolv
 		if e.cteTableExists(ref.Name) {
 			continue
 		}
+		// A virtual catalog relation has no durable schema: its columns come from
+		// catalog metadata, and asking the session for them from inside a locked
+		// scan would deadlock on the session lock the scan already holds.
+		if _, ok := e.catalogRelationFor(ref); ok {
+			continue
+		}
 		schema, err := e.session.GetSchema(ref.Name)
 		if err != nil {
 			continue

+ 62 - 0
pkg/pgserver/pg_catalog_test.go

@@ -1,9 +1,14 @@
 package pgserver
 
 import (
+	"net"
 	"strconv"
 	"strings"
 	"testing"
+	"time"
+
+	"github.com/danfragoso/pizzasql-next/pkg/storage"
+	"github.com/danfragoso/pizzasql-next/pkg/testkv"
 )
 
 func simpleQuery(sql string) *Message {
@@ -154,3 +159,60 @@ func TestPgCatalogRegprocWireType(t *testing.T) {
 		t.Fatalf("pg_type regproc values = %v, want 0/0", rows)
 	}
 }
+
+// A catalog-backed predicate in a write must not deadlock the session: the
+// engine resolves the snapshot before the locked scan. A deadlock here leaks the
+// table gate, so a second connection is probed for reads and writes after the
+// error, bounded by a read deadline.
+func TestPgCatalogWritePredicateKeepsSecondSessionUsable(t *testing.T) {
+	kv := testkv.New(t)
+	pool := kv.Pool(8)
+	t.Cleanup(func() { pool.Close() })
+	schema := storage.NewSchemaManager(pool, "pg_catalog_lock")
+	table := storage.NewTableManager(pool, schema, "pg_catalog_lock")
+
+	writer, writerClient := newDataConnectionFor(t, schema, table)
+	reader, readerClient := newDataConnectionFor(t, schema, table)
+
+	run := func(c *Connection, client net.Conn, sql string) []*Message {
+		t.Helper()
+		// A blocked statement never answers; bound the wait so a regression
+		// fails instead of hanging the suite.
+		if err := client.SetReadDeadline(time.Now().Add(10 * time.Second)); err != nil {
+			t.Fatalf("set deadline: %v", err)
+		}
+		defer func() { _ = client.SetReadDeadline(time.Time{}) }()
+		return runQuery(t, c, client, simpleQuery(sql))
+	}
+
+	for _, sql := range []string{
+		"CREATE TABLE locked_write (id INTEGER PRIMARY KEY, v TEXT NOT NULL)",
+		"INSERT INTO locked_write VALUES (1, 'a')",
+	} {
+		if msgs := run(writer, writerClient, sql); errorCode(msgs[0]) != "" {
+			t.Fatalf("%s: %s", sql, errorMessage(msgs[0]))
+		}
+	}
+
+	msgs := run(writer, writerClient, `UPDATE locked_write SET v = 'z' WHERE v = 'a' AND 'missing_relation'::regclass IS NOT NULL`)
+	msg := findError(msgs)
+	if msg == nil {
+		t.Fatalf("expected an error for the unknown regclass, got %v", msgs)
+	}
+	if code := errorCode(msg); code != ErrCodeUndefinedTable {
+		t.Fatalf("SQLSTATE = %q, want %q (%s)", code, ErrCodeUndefinedTable, errorMessage(msg))
+	}
+
+	// The failed statement left no partial write and released its locks.
+	msgs = run(reader, readerClient, `SELECT count(*) FROM locked_write WHERE v = 'z'`)
+	if msg := findError(msgs); msg != nil {
+		t.Fatalf("second session SELECT failed: %s", errorMessage(msg))
+	}
+	if rows := catalogRows(t, msgs); len(rows) != 1 || rows[0][0] != "0" {
+		t.Fatalf("updated rows = %v, want 0", rows)
+	}
+	msgs = run(reader, readerClient, `INSERT INTO locked_write VALUES (2, 'b')`)
+	if msg := findError(msgs); msg != nil {
+		t.Fatalf("second session INSERT failed: %s", errorMessage(msg))
+	}
+}