2
0
Эх сурвалжийг харах

fix: restore compatible Describe behavior for extended clients (#27)

Danilo Fragoso 16 цаг өмнө
parent
commit
7ee869ae24

+ 1 - 1
README.md

@@ -321,7 +321,7 @@ pgcli postgresql://localhost:5432/pizzasql
    - Use `INTEGER PRIMARY KEY AUTOINCREMENT`, not `SERIAL`
    - Use `TEXT`, not `VARCHAR` with enforced length
 2. **Parameter placeholders**: use `$1, $2...` (PostgreSQL style) or `?` (SQLite style). PG Bind rewrites `$n` only outside quoted regions and line/block comments. Initialization probes such as `SELECT version()` and `SHOW server_version` match complete token sequences, never substrings inside SQL literals or unrelated projections.
-3. **Extended protocol scope**: Parse accepts one command; Bind requests must use text parameters/all-text results. Describe, binary results, and nonzero Execute row limits fail with SQLSTATE `0A000` before executing side effects. Portals are one-shot, not resumable. Unsupported binary parameter OIDs also report `0A000`.
+3. **Extended protocol scope**: Parse accepts one command; Bind requests must use text parameters/all-text results. `Describe` returns `ParameterDescription` then `NoData` without executing anything; `Execute` still sends the real `RowDescription`. Binary results and nonzero Execute row limits fail with SQLSTATE `0A000` before side effects. Portals are one-shot, not resumable. Unsupported binary parameter OIDs also report `0A000`.
 4. **No PostgreSQL-specific features**: no schemas, roles, `ARRAY`, `JSONB`, etc.
 
 ---

+ 1 - 1
pkg/capabilities/capabilities.go

@@ -146,7 +146,7 @@ func Current() *Document {
 			SQLiteCompat:   "3.35.0",
 			SimpleQuery:    true,
 			ExtendedQuery:  true,
-			Describe:       Feature{Supported: false, Status: "rejected", SQLState: "0A000", Notes: "use simple query or Parse/Bind/Execute without Describe; no speculative execution for metadata"},
+			Describe:       Feature{Supported: true, Status: "compatibility_no_data", Notes: "statement Describe returns ParameterDescription then NoData; portal Describe returns NoData; Execute still emits the real RowDescription"},
 			BinaryResults:  Feature{Supported: false, Status: "rejected", SQLState: "0A000", Notes: "all-text result formats only; per-column format lists are also rejected"},
 			BoundedExecute: Feature{Supported: false, Status: "rejected", SQLState: "0A000", Notes: "Execute maxRows must be zero; portals are one-shot, not resumable"},
 			ProxyErrors:    true,

+ 14 - 5
pkg/pgserver/connection.go

@@ -578,9 +578,18 @@ func (c *Connection) handleDescribe(msg *Message) error {
 	}
 	switch msg.Data[0] {
 	case 'S':
-		if _, ok := c.statements[name]; !ok {
+		statement, ok := c.statements[name]
+		if !ok {
 			return c.failExtended(ErrCodeInvalidParameter, fmt.Errorf("prepared statement %q does not exist", name))
 		}
+		mb := NewMessageBuilder()
+		mb.WriteInt16(int16(len(statement.paramOIDs)))
+		for _, oid := range statement.paramOIDs {
+			mb.WriteInt32(oid)
+		}
+		if err := c.writeMessage(MsgParameterDescription, mb.Bytes()); err != nil {
+			return err
+		}
 	case 'P':
 		if _, ok := c.portals[name]; !ok {
 			return c.failExtended(ErrCodeInvalidParameter, fmt.Errorf("portal %q does not exist", name))
@@ -588,10 +597,10 @@ func (c *Connection) handleDescribe(msg *Message) error {
 	default:
 		return c.failExtended(ErrCodeProtocolViolation, fmt.Errorf("invalid Describe target"))
 	}
-	// Do not claim NoData for row-producing statements or execute queries to
-	// discover metadata. Clients must avoid Describe until side-effect-free
-	// parameter/row description is implemented.
-	return c.failExtended(ErrCodeFeatureNotSupported, fmt.Errorf("Describe metadata is not supported; use simple query or Parse/Bind/Execute without Describe"))
+	// Row metadata requires analysis without side effects. Return NoData for
+	// compatibility; Execute still emits the real RowDescription. This is the
+	// pre-0.4.0 behavior used by common extended-protocol clients.
+	return c.writeMessage(MsgNoData, nil)
 }
 
 func (c *Connection) handleExecute(msg *Message) error {

+ 76 - 9
pkg/pgserver/protocol_guards_test.go

@@ -44,8 +44,6 @@ func TestUnsupportedExtendedRequestsFailBeforeSideEffects(t *testing.T) {
 		{"invalid_result_format", &Message{Type: MsgBind, Data: guardBind(nil, []int16{2})}, ErrCodeProtocolViolation},
 		{"invalid_parameter_format", &Message{Type: MsgBind, Data: guardBind([]int16{2}, nil)}, ErrCodeProtocolViolation},
 		{"parameter_format_count", &Message{Type: MsgBind, Data: guardBind([]int16{0, 0}, nil)}, ErrCodeProtocolViolation},
-		{"describe_statement", &Message{Type: MsgDescribe, Data: []byte{'S', 'q', 0}}, ErrCodeFeatureNotSupported},
-		{"describe_portal", &Message{Type: MsgDescribe, Data: []byte{'P', 'p', 0}}, ErrCodeFeatureNotSupported},
 		{"bounded_execute", &Message{Type: MsgExecute, Data: []byte{'p', 0, 0, 0, 0, 1}}, ErrCodeFeatureNotSupported},
 		{"negative_execute", &Message{Type: MsgExecute, Data: []byte{'p', 0, 255, 255, 255, 255}}, ErrCodeProtocolViolation},
 	} {
@@ -75,12 +73,16 @@ func TestUnsupportedExtendedRequestsFailBeforeSideEffects(t *testing.T) {
 	}
 }
 
-func TestDescribeFailurePreservesFailedTransactionState(t *testing.T) {
+func TestDescribeDoesNotExecuteOrFailTransaction(t *testing.T) {
 	c, conn := guardConnection(t)
 	if err := c.handleMessage(&Message{Type: MsgQuery, Data: []byte("BEGIN\x00")}); err != nil {
 		t.Fatal(err)
 	}
+	c.statements["q"] = &preparedStatement{query: "INSERT INTO t VALUES (1) RETURNING id", paramOIDs: []int32{20}}
 	c.portals["p"] = &portal{query: "INSERT INTO t VALUES (1) RETURNING id"}
+	if err := c.handleMessage(&Message{Type: MsgDescribe, Data: []byte{'S', 'q', 0}}); err != nil {
+		t.Fatal(err)
+	}
 	if err := c.handleMessage(&Message{Type: MsgDescribe, Data: []byte{'P', 'p', 0}}); err != nil {
 		t.Fatal(err)
 	}
@@ -88,16 +90,29 @@ func TestDescribeFailurePreservesFailedTransactionState(t *testing.T) {
 		t.Fatal(err)
 	}
 	messages := readAllMessages(t, conn.bytes())
-	last := messages[len(messages)-1]
-	if last.Type != MsgReadyForQuery || len(last.Data) != 1 || last.Data[0] != TxStatusFailed {
-		t.Fatalf("Describe did not fail transaction: %v", last)
+	seenParam, seenNoData := false, 0
+	for _, message := range messages {
+		switch message.Type {
+		case MsgParameterDescription:
+			seenParam = true
+		case MsgNoData:
+			seenNoData++
+		case MsgErrorResponse:
+			t.Fatalf("Describe failed: %s", errorCode(message))
+		}
+	}
+	if !seenParam || seenNoData != 2 {
+		t.Fatalf("Describe messages = %v", messages)
+	}
+	if c.txStatus != TxStatusInBlock || c.extendedFailed {
+		t.Fatalf("Describe changed transaction state: %c failed=%v", c.txStatus, c.extendedFailed)
+	}
+	if result, err := c.executor.Execute(parseStmt(t, "SELECT COUNT(*) FROM t")); err != nil || result.Rows[0][0] != int64(0) {
+		t.Fatalf("Describe executed a side effect: %v %v", result, err)
 	}
 	if err := c.handleMessage(&Message{Type: MsgQuery, Data: []byte("ROLLBACK\x00")}); err != nil {
 		t.Fatal(err)
 	}
-	if c.txStatus != TxStatusIdle {
-		t.Fatal("rollback did not recover")
-	}
 }
 
 func TestParseRejectsMultipleCommands(t *testing.T) {
@@ -117,3 +132,55 @@ func TestParseRejectsMultipleCommands(t *testing.T) {
 		t.Fatal(messages)
 	}
 }
+
+func TestTypicalExtendedSequenceWithDescribeSucceeds(t *testing.T) {
+	c, conn := guardConnection(t)
+	if _, err := c.executor.Execute(parseStmt(t, "INSERT INTO t VALUES (7)")); err != nil {
+		t.Fatal(err)
+	}
+	parse := NewMessageBuilder()
+	parse.WriteString("")
+	parse.WriteString("SELECT id FROM t WHERE id = $1")
+	parse.WriteInt16(1)
+	parse.WriteInt32(20)
+	bind := NewMessageBuilder()
+	bind.WriteString("")
+	bind.WriteString("")
+	bind.WriteInt16(0)
+	bind.WriteInt16(1)
+	bind.WriteInt32(1)
+	bind.WriteBytes([]byte("7"))
+	bind.WriteInt16(0)
+	execute := NewMessageBuilder()
+	execute.WriteString("")
+	execute.WriteInt32(0)
+	for _, msg := range []*Message{
+		{Type: MsgParse, Data: parse.Bytes()},
+		{Type: MsgBind, Data: bind.Bytes()},
+		{Type: MsgDescribe, Data: []byte{'P', 0}},
+		{Type: MsgExecute, Data: execute.Bytes()},
+		{Type: MsgSync},
+	} {
+		if err := c.handleMessage(msg); err != nil {
+			t.Fatal(err)
+		}
+	}
+	messages := readAllMessages(t, conn.bytes())
+	var types []byte
+	var row []string
+	for _, message := range messages {
+		types = append(types, message.Type)
+		if message.Type == MsgErrorResponse {
+			t.Fatalf("extended SELECT failed: %s", errorCode(message))
+		}
+		if message.Type == MsgDataRow {
+			row = dataRowValues(t, message)
+		}
+	}
+	if string(types) != "12nTDCZ" {
+		t.Fatalf("message sequence = %q", types)
+	}
+	if len(row) != 1 || row[0] != "7" {
+		t.Fatalf("row = %v", row)
+	}
+}