Merge #38586

38586: pgwire: add some portal tests r=mjibson a=mjibson



Co-authored-by: Matt Jibson <[email protected]>

pkg/sql/pgwire/testdata/pgtest/portals

@@ -0,0 +1,116 @@
+# Verify that a completed portal can't be re-executed.
+
+send
+Parse {"Query": "SELECT 1"}
+Bind
+Execute
+Sync
+----
+
+until
+ReadyForQuery
+----
+{"Type":"ParseComplete"}
+{"Type":"BindComplete"}
+{"Type":"DataRow","Values":[{"text":"1"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+send
+Execute
+Sync
+----
+
+until
+ErrorResponse
+ReadyForQuery
+----
+{"Type":"ErrorResponse"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+# Verify that closing a bound portal prevents execution.
+
+# 80 = ASCII 'P'
+send
+Parse {"Name": "s", "Query": "SELECT 1"}
+Bind {"DestinationPortal": "p", "PreparedStatement": "s"}
+Close {"ObjectType": 80, "Name": "p"}
+Execute {"Portal": "p"}
+Sync
+----
+
+until
+ErrorResponse
+ReadyForQuery
+----
+{"Type":"ParseComplete"}
+{"Type":"BindComplete"}
+{"Type":"CloseComplete"}
+{"Type":"ErrorResponse"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+# The spec says that closing a prepared statement also closes its portals,
+# but that doesn't seem to be the case. Below I would expect that Bind,
+# Close, Execute causes the execute to return an error, but it instead
+# returns the portal result. This happens in both Postgres and Cockroach.
+
+# 83 = ASCII 'S'
+# After closing, re-parse with the same name to make sure the execute
+# happens on the old statement.
+send
+Bind {"DestinationPortal": "p", "PreparedStatement": "s"}
+Close {"ObjectType": 83, "Name": "s"}
+Parse {"Name": "s", "Query": "SELECT 2"}
+Execute {"Portal": "p"}
+Sync
+----
+
+until
+ReadyForQuery
+----
+{"Type":"BindComplete"}
+{"Type":"CloseComplete"}
+{"Type":"ParseComplete"}
+{"Type":"DataRow","Values":[{"text":"1"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"I"}
+
+# Portal still isn't destroyed within a transaction either, in PG or CR.
+
+send
+Query {"String": "BEGIN"}
+----
+
+until
+ReadyForQuery
+----
+{"Type":"CommandComplete","CommandTag":"BEGIN"}
+{"Type":"ReadyForQuery","TxStatus":"T"}
+
+send
+Bind {"DestinationPortal": "p", "PreparedStatement": "s"}
+Close {"ObjectType": 83, "Name": "s"}
+Parse {"Name": "s", "Query": "SELECT 3"}
+Execute {"Portal": "p"}
+Sync
+----
+
+until
+ReadyForQuery
+----
+{"Type":"BindComplete"}
+{"Type":"CloseComplete"}
+{"Type":"ParseComplete"}
+{"Type":"DataRow","Values":[{"text":"2"}]}
+{"Type":"CommandComplete","CommandTag":"SELECT 1"}
+{"Type":"ReadyForQuery","TxStatus":"T"}
+
+send
+Query {"String": "COMMIT"}
+----
+
+until
+ReadyForQuery
+----
+{"Type":"CommandComplete","CommandTag":"COMMIT"}
+{"Type":"ReadyForQuery","TxStatus":"I"}

pkg/sql/prepared_stmt.go

@@ -100,12 +100,6 @@ type preparedStatementsAccessor interface {
 // Note that PreparedPortals maintain a reference counter internally.
 // References need to be registered with incRef() and de-registered with
 // decRef().
-//
-// TODO(andrei): In Postgres, portals can only be used to "execute a query" once
-// (but they allow one to move back and forth through the results). Our portals
-// can be used to execute a query multiple times, which is a bug (executing an
-// exhausted portal in Postres returns 0 results; in CRDB executing a portal a
-// second time always restarts the query).
 type PreparedPortal struct {
 	Stmt  *PreparedStatement
 	Qargs tree.QueryArguments

pkg/testutils/pgtest/datadriven.go

@@ -134,6 +134,8 @@ func toMessage(typ string) interface{} {
 	switch typ {
 	case "Bind":
 		return &pgproto3.Bind{}
+	case "Close":
+		return &pgproto3.Close{}
 	case "CommandComplete":
 		return &pgproto3.CommandComplete{}
 	case "DataRow":

pkg/testutils/pgtest/pgtest.go

@@ -12,8 +12,10 @@ package pgtest
 
 import (
 	"context"
+	"fmt"
 	"net"
 	"reflect"
+	"testing"
 
 	"github.com/jackc/pgx/pgproto3"
 	"github.com/pkg/errors"
@@ -73,6 +75,9 @@ func (p *PGTest) Close() error {
 
 // Send sends msg to the serrver.
 func (p *PGTest) Send(msg pgproto3.FrontendMessage) error {
+	if testing.Verbose() {
+		fmt.Printf("SEND %T: %+[1]v\n", msg)
+	}
 	return p.fe.Send(msg)
 }
 
@@ -104,6 +109,9 @@ func (p *PGTest) Until(typs ...pgproto3.BackendMessage) ([]pgproto3.BackendMessa
 		if err != nil {
 			return nil, errors.Wrap(err, "receive")
 		}
+		if testing.Verbose() {
+			fmt.Printf("RECV %T: %+[1]v\n", recv)
+		}
 		if errmsg, ok := recv.(*pgproto3.ErrorResponse); ok && typ != typErrorResponse {
 			return nil, errors.Errorf("waiting for %T, got %#v", typs[0], errmsg)
 		}