Przeglądaj źródła

fix(parser): reject SELECT ... FOR UPDATE with SQLSTATE 0A000

FOR is not a token in this lexer, so the clause was previously consumed as a
table alias and produced a confusing follow-on syntax error. Detect locking
clauses (FOR UPDATE/SHARE/NO KEY UPDATE, with NOWAIT or SKIP LOCKED) at the end
of SELECT and compound SELECT parsing and return a feature-not-supported error
that the pg wire and HTTP layers can map to 0A000.

Task: #67
PizzaSQL Bot 1 dzień temu
rodzic
commit
abd54e857e

+ 39 - 0
pkg/parser/locking_test.go

@@ -0,0 +1,39 @@
+package parser
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/danfragoso/pizzasql-next/pkg/lexer"
+	"github.com/danfragoso/pizzasql-next/pkg/pgerror"
+)
+
+func TestSelectLockingClauseRejected(t *testing.T) {
+	for _, sql := range []string{
+		"SELECT * FROM t FOR UPDATE",
+		"SELECT * FROM t WHERE id = 1 FOR UPDATE NOWAIT",
+		"SELECT * FROM t FOR SHARE",
+		"SELECT * FROM t FOR NO KEY UPDATE SKIP LOCKED",
+		"SELECT id FROM t ORDER BY id LIMIT 10 FOR UPDATE",
+		"SELECT id FROM t UNION SELECT id FROM u FOR UPDATE",
+	} {
+		t.Run(sql, func(t *testing.T) {
+			_, err := New(lexer.New(sql)).Parse()
+			if !errors.Is(err, pgerror.ErrFeatureNotSupported) {
+				t.Fatalf("error = %v, want ErrFeatureNotSupported", err)
+			}
+		})
+	}
+}
+
+func TestSelectWithoutLockingClauseStillParses(t *testing.T) {
+	for _, sql := range []string{
+		"SELECT * FROM t",
+		"SELECT * FROM t WHERE id = 1",
+		"SELECT * FROM t ORDER BY id LIMIT 1",
+	} {
+		if _, err := New(lexer.New(sql)).Parse(); err != nil {
+			t.Fatalf("%s: %v", sql, err)
+		}
+	}
+}

+ 36 - 1
pkg/parser/parser.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	"github.com/danfragoso/pizzasql-next/pkg/lexer"
+	"github.com/danfragoso/pizzasql-next/pkg/pgerror"
 )
 
 // Parser parses SQL statements into an AST.
@@ -286,6 +287,10 @@ func (p *Parser) parseSelect() (*SelectStmt, error) {
 		stmt.Offset = offset
 	}
 
+	if err := p.rejectLockingClause(); err != nil {
+		return nil, err
+	}
+
 	return stmt, nil
 }
 
@@ -421,6 +426,9 @@ func (p *Parser) parseCompoundChain(first *SelectStmt) (*CompoundSelect, error)
 		}
 		compound.Offset = offset
 	}
+	if err := p.rejectLockingClause(); err != nil {
+		return nil, err
+	}
 
 	return compound, nil
 }
@@ -606,7 +614,7 @@ func (p *Parser) parseTableRef() (*TableRef, error) {
 		}
 		ref.Alias = p.curToken.Literal
 		p.nextToken()
-	} else if p.curTokenIs(lexer.TokenIdent) && !p.isClauseKeyword() {
+	} else if p.curTokenIs(lexer.TokenIdent) && !p.isClauseKeyword() && !p.isLockingClauseStart() {
 		ref.Alias = p.curToken.Literal
 		p.nextToken()
 	}
@@ -635,6 +643,33 @@ func (p *Parser) isClauseKeyword() bool {
 	return false
 }
 
+// isLockingClauseStart reports whether the current identifier starts a
+// PostgreSQL row-locking clause. FOR is not a reserved keyword in this lexer,
+// so without this guard "FROM t FOR UPDATE" would treat FOR as a table alias.
+func (p *Parser) isLockingClauseStart() bool {
+	return p.curTokenIs(lexer.TokenIdent) && strings.EqualFold(p.curToken.Literal, "FOR")
+}
+
+// rejectLockingClause rejects SELECT ... FOR UPDATE/SHARE and its NOWAIT / SKIP
+// LOCKED variants. The engine has no row locks; claiming support by parsing the
+// clause would silently provide weaker isolation than PostgreSQL.
+func (p *Parser) rejectLockingClause() error {
+	if !p.isLockingClauseStart() {
+		return nil
+	}
+	locking := p.peekTokenIs(lexer.TokenUPDATE) ||
+		(p.peekTokenIs(lexer.TokenIdent) &&
+			(strings.EqualFold(p.peekToken.Literal, "SHARE") ||
+				strings.EqualFold(p.peekToken.Literal, "NO") ||
+				strings.EqualFold(p.peekToken.Literal, "KEY")))
+	if !locking {
+		return nil
+	}
+	return pgerror.Unsupported(
+		"SELECT ... FOR UPDATE/SHARE row locking (including NOWAIT and SKIP LOCKED) is not supported; use the documented optimistic transaction model with SQLSTATE 40001 retries instead",
+	)
+}
+
 func (p *Parser) parseJoin() (*JoinClause, error) {
 	join := &JoinClause{Type: JoinInner}
 

+ 45 - 0
pkg/pgserver/unsupported_locking_test.go

@@ -0,0 +1,45 @@
+package pgserver
+
+import (
+	"bytes"
+	"testing"
+)
+
+// errorField extracts a named field from an ErrorResponse.
+func errorField(t *testing.T, msg *Message, want byte) string {
+	t.Helper()
+	if msg == nil || msg.Type != MsgErrorResponse {
+		t.Fatalf("message type = %v, want ErrorResponse", msg)
+	}
+	data := msg.Data
+	for i := 0; i < len(data); {
+		if data[i] == 0 {
+			break
+		}
+		field := data[i]
+		i++
+		end := bytes.IndexByte(data[i:], 0)
+		if end < 0 {
+			break
+		}
+		value := string(data[i : i+end])
+		if field == want {
+			return value
+		}
+		i += end + 1
+	}
+	return ""
+}
+
+func TestLockingClauseRejectedOverSimpleQuery(t *testing.T) {
+	c, client := newDataConnection(t)
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("CREATE TABLE t (id INTEGER PRIMARY KEY)"), 0)})
+
+	msgs := runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("SELECT * FROM t FOR UPDATE"), 0)})
+	if code := errorCode(msgs[0]); code != ErrCodeFeatureNotSupported {
+		t.Fatalf("error code = %q, want %q", code, ErrCodeFeatureNotSupported)
+	}
+	if message := errorField(t, msgs[0], ErrorFieldMessage); !bytes.Contains([]byte(message), []byte("row locking")) {
+		t.Fatalf("error message = %q, want a row-locking explanation", message)
+	}
+}