diff --git a/pkg/sql/pgwire/testdata/pgtest/portals b/pkg/sql/pgwire/testdata/pgtest/portals new file mode 100644 index 000000000000..67f082bb0c6c --- /dev/null +++ b/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"} diff --git a/pkg/sql/prepared_stmt.go b/pkg/sql/prepared_stmt.go index e1c6dc40c765..9e29f52ec97f 100644 --- a/pkg/sql/prepared_stmt.go +++ b/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 diff --git a/pkg/testutils/pgtest/datadriven.go b/pkg/testutils/pgtest/datadriven.go index c7639644dd60..9cd40a18f7b9 100644 --- a/pkg/testutils/pgtest/datadriven.go +++ b/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": diff --git a/pkg/testutils/pgtest/pgtest.go b/pkg/testutils/pgtest/pgtest.go index 688821da7a12..b94a1eccc4d0 100644 --- a/pkg/testutils/pgtest/pgtest.go +++ b/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) }