From 66625e6489b729cb4ba484f9931e5bdb180971da Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Sat, 20 Apr 2019 11:47:16 -0500 Subject: [PATCH] Prepare takes context Also remove PrepareEx. It's primary usage was for context. Supplying parameter OIDs is unnecessary when you can type cast in the query SQL. If it does become necessary or desirable to add options back it can be added in a backwards compatible way by adding a varargs as last argument. --- batch_test.go | 2 +- bench-tmp_test.go | 4 ++-- bench_test.go | 10 +++++----- conn.go | 30 +++-------------------------- conn_test.go | 38 +++++-------------------------------- copy_from.go | 2 +- pgtype/hstore_array_test.go | 2 +- pgtype/record_test.go | 2 +- pgtype/testutil/testutil.go | 4 ++-- stdlib/sql.go | 4 ++-- tx.go | 9 ++------- 11 files changed, 25 insertions(+), 82 deletions(-) diff --git a/batch_test.go b/batch_test.go index 3831a678..8709f17d 100644 --- a/batch_test.go +++ b/batch_test.go @@ -165,7 +165,7 @@ func TestConnBeginBatchWithPreparedStatement(t *testing.T) { conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE")) defer closeConn(t, conn) - _, err := conn.Prepare("ps1", "select n from generate_series(0,$1::int) n") + _, err := conn.Prepare(context.Background(), "ps1", "select n from generate_series(0,$1::int) n") if err != nil { t.Fatal(err) } diff --git a/bench-tmp_test.go b/bench-tmp_test.go index 787fd162..963ef2b3 100644 --- a/bench-tmp_test.go +++ b/bench-tmp_test.go @@ -10,7 +10,7 @@ func BenchmarkPgtypeInt4ParseBinary(b *testing.B) { conn := mustConnectString(b, os.Getenv("PGX_TEST_DATABASE")) defer closeConn(b, conn) - _, err := conn.Prepare("selectBinary", "select n::int4 from generate_series(1, 100) n") + _, err := conn.Prepare(context.Background(), "selectBinary", "select n::int4 from generate_series(1, 100) n") if err != nil { b.Fatal(err) } @@ -41,7 +41,7 @@ func BenchmarkPgtypeInt4EncodeBinary(b *testing.B) { conn := mustConnectString(b, os.Getenv("PGX_TEST_DATABASE")) defer closeConn(b, conn) - _, err := conn.Prepare("encodeBinary", "select $1::int4, $2::int4, $3::int4, $4::int4, $5::int4, $6::int4, $7::int4") + _, err := conn.Prepare(context.Background(), "encodeBinary", "select $1::int4, $2::int4, $3::int4, $4::int4, $5::int4, $6::int4, $7::int4") if err != nil { b.Fatal(err) } diff --git a/bench_test.go b/bench_test.go index a47d6b51..f0d65419 100644 --- a/bench_test.go +++ b/bench_test.go @@ -17,7 +17,7 @@ func BenchmarkPointerPointerWithNullValues(b *testing.B) { conn := mustConnect(b, mustParseConfig(b, os.Getenv("PGX_TEST_DATABASE"))) defer closeConn(b, conn) - _, err := conn.Prepare("selectNulls", "select 1::int4, 'johnsmith', null::text, null::text, null::text, null::date, null::timestamptz") + _, err := conn.Prepare(context.Background(), "selectNulls", "select 1::int4, 'johnsmith', null::text, null::text, null::text, null::date, null::timestamptz") if err != nil { b.Fatal(err) } @@ -77,7 +77,7 @@ func BenchmarkPointerPointerWithPresentValues(b *testing.B) { conn := mustConnect(b, mustParseConfig(b, os.Getenv("PGX_TEST_DATABASE"))) defer closeConn(b, conn) - _, err := conn.Prepare("selectNulls", "select 1::int4, 'johnsmith', 'johnsmith@example.com', 'John Smith', 'male', '1970-01-01'::date, '2015-01-01 00:00:00'::timestamptz") + _, err := conn.Prepare(context.Background(), "selectNulls", "select 1::int4, 'johnsmith', 'johnsmith@example.com', 'John Smith', 'male', '1970-01-01'::date, '2015-01-01 00:00:00'::timestamptz") if err != nil { b.Fatal(err) } @@ -189,7 +189,7 @@ func BenchmarkSelectWithLoggingErrorWithDiscard(b *testing.B) { } func benchmarkSelectWithLog(b *testing.B, conn *pgx.Conn) { - _, err := conn.Prepare("test", "select 1::int4, 'johnsmith', 'johnsmith@example.com', 'John Smith', 'male', '1970-01-01'::date, '2015-01-01 00:00:00'::timestamptz") + _, err := conn.Prepare(context.Background(), "test", "select 1::int4, 'johnsmith', 'johnsmith@example.com', 'John Smith', 'male', '1970-01-01'::date, '2015-01-01 00:00:00'::timestamptz") if err != nil { b.Fatal(err) } @@ -339,7 +339,7 @@ func benchmarkWriteNRowsViaInsert(b *testing.B, n int) { defer closeConn(b, conn) mustExec(b, conn, benchmarkWriteTableCreateSQL) - _, err := conn.Prepare("insert_t", benchmarkWriteTableInsertSQL) + _, err := conn.Prepare(context.Background(), "insert_t", benchmarkWriteTableInsertSQL) if err != nil { b.Fatal(err) } @@ -450,7 +450,7 @@ func benchmarkWriteNRowsViaMultiInsert(b *testing.B, n int) { defer closeConn(b, conn) mustExec(b, conn, benchmarkWriteTableCreateSQL) - _, err := conn.Prepare("insert_t", benchmarkWriteTableInsertSQL) + _, err := conn.Prepare(context.Background(), "insert_t", benchmarkWriteTableInsertSQL) if err != nil { b.Fatal(err) } diff --git a/conn.go b/conn.go index 40df37c5..67ca50bc 100644 --- a/conn.go +++ b/conn.go @@ -216,18 +216,7 @@ func (c *Conn) ParameterStatus(key string) string { // Prepare is idempotent; i.e. it is safe to call Prepare multiple times with the same // name and sql arguments. This allows a code path to Prepare and Query/Exec without // concern for if the statement has already been prepared. -func (c *Conn) Prepare(name, sql string) (ps *PreparedStatement, err error) { - return c.PrepareEx(context.Background(), name, sql, nil) -} - -// PrepareEx creates a prepared statement with name and sql. sql can contain placeholders -// for bound parameters. These placeholders are referenced positional as $1, $2, etc. -// It differs from Prepare as it allows additional options (such as parameter OIDs) to be passed via struct -// -// PrepareEx is idempotent; i.e. it is safe to call PrepareEx multiple times with the same -// name and sql arguments. This allows a code path to PrepareEx and Query/Exec without -// concern for if the statement has already been prepared. -func (c *Conn) PrepareEx(ctx context.Context, name, sql string, opts *PrepareExOptions) (ps *PreparedStatement, err error) { +func (c *Conn) Prepare(ctx context.Context, name, sql string) (ps *PreparedStatement, err error) { if name != "" { if ps, ok := c.preparedStatements[name]; ok && ps.SQL == sql { return ps, nil @@ -237,25 +226,12 @@ func (c *Conn) PrepareEx(ctx context.Context, name, sql string, opts *PrepareExO if c.shouldLog(LogLevelError) { defer func() { if err != nil { - c.log(LogLevelError, "prepareEx failed", map[string]interface{}{"err": err, "name": name, "sql": sql}) + c.log(LogLevelError, "Prepare failed", map[string]interface{}{"err": err, "name": name, "sql": sql}) } }() } - if opts == nil { - opts = &PrepareExOptions{} - } - - if len(opts.ParameterOIDs) > 65535 { - return nil, errors.Errorf("Number of PrepareExOptions ParameterOIDs must be between 0 and 65535, received %d", len(opts.ParameterOIDs)) - } - - var paramOIDs []uint32 - for _, oid := range opts.ParameterOIDs { - paramOIDs = append(paramOIDs, uint32(oid)) - } - - psd, err := c.pgConn.Prepare(context.TODO(), name, sql, paramOIDs) + psd, err := c.pgConn.Prepare(context.TODO(), name, sql, nil) if err != nil { return nil, err } diff --git a/conn_test.go b/conn_test.go index 93c4ad42..ba4f038d 100644 --- a/conn_test.go +++ b/conn_test.go @@ -281,7 +281,7 @@ func TestPrepare(t *testing.T) { conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE")) defer closeConn(t, conn) - _, err := conn.Prepare("test", "select $1::varchar") + _, err := conn.Prepare(context.Background(), "test", "select $1::varchar") if err != nil { t.Errorf("Unable to prepare statement: %v", err) return @@ -305,7 +305,7 @@ func TestPrepare(t *testing.T) { // Create another prepared statement to ensure Deallocate left the connection // in a working state and that we can reuse the prepared statement name. - _, err = conn.Prepare("test", "select $1::integer") + _, err = conn.Prepare(context.Background(), "test", "select $1::integer") if err != nil { t.Errorf("Unable to prepare statement: %v", err) return @@ -333,7 +333,7 @@ func TestPrepareBadSQLFailure(t *testing.T) { conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE")) defer closeConn(t, conn) - if _, err := conn.Prepare("badSQL", "select foo"); err == nil { + if _, err := conn.Prepare(context.Background(), "badSQL", "select foo"); err == nil { t.Fatal("Prepare should have failed with syntax error") } @@ -347,7 +347,7 @@ func TestPrepareIdempotency(t *testing.T) { defer closeConn(t, conn) for i := 0; i < 2; i++ { - _, err := conn.Prepare("test", "select 42::integer") + _, err := conn.Prepare(context.Background(), "test", "select 42::integer") if err != nil { t.Fatalf("%d. Unable to prepare statement: %v", i, err) } @@ -363,41 +363,13 @@ func TestPrepareIdempotency(t *testing.T) { } } - _, err := conn.Prepare("test", "select 'fail'::varchar") + _, err := conn.Prepare(context.Background(), "test", "select 'fail'::varchar") if err == nil { t.Fatalf("Prepare statement with same name but different SQL should have failed but it didn't") return } } -func TestPrepareEx(t *testing.T) { - t.Parallel() - - conn := mustConnectString(t, os.Getenv("PGX_TEST_DATABASE")) - defer closeConn(t, conn) - - _, err := conn.PrepareEx(context.Background(), "test", "select $1", &pgx.PrepareExOptions{ParameterOIDs: []pgtype.OID{pgtype.TextOID}}) - if err != nil { - t.Errorf("Unable to prepare statement: %v", err) - return - } - - var s string - err = conn.QueryRow(context.Background(), "test", "hello").Scan(&s) - if err != nil { - t.Errorf("Executing prepared statement failed: %v", err) - } - - if s != "hello" { - t.Errorf("Prepared statement did not return expected value: %v", s) - } - - err = conn.Deallocate(context.Background(), "test") - if err != nil { - t.Errorf("conn.Deallocate failed: %v", err) - } -} - func TestFatalRxError(t *testing.T) { t.Parallel() diff --git a/copy_from.go b/copy_from.go index 08c9488b..f69ec584 100644 --- a/copy_from.go +++ b/copy_from.go @@ -68,7 +68,7 @@ func (ct *copyFrom) run(ctx context.Context) (int, error) { } quotedColumnNames := cbuf.String() - ps, err := ct.conn.Prepare("", fmt.Sprintf("select %s from %s", quotedColumnNames, quotedTableName)) + ps, err := ct.conn.Prepare(ctx, "", fmt.Sprintf("select %s from %s", quotedColumnNames, quotedTableName)) if err != nil { return 0, err } diff --git a/pgtype/hstore_array_test.go b/pgtype/hstore_array_test.go index 03dc2ff1..849b5835 100644 --- a/pgtype/hstore_array_test.go +++ b/pgtype/hstore_array_test.go @@ -70,7 +70,7 @@ func TestHstoreArrayTranscode(t *testing.T) { Status: pgtype.Present, } - ps, err := conn.Prepare("test", "select $1::hstore[]") + ps, err := conn.Prepare(context.Background(), "test", "select $1::hstore[]") if err != nil { t.Fatal(err) } diff --git a/pgtype/record_test.go b/pgtype/record_test.go index 44b0e9d8..a4fc1e5d 100644 --- a/pgtype/record_test.go +++ b/pgtype/record_test.go @@ -85,7 +85,7 @@ func TestRecordTranscode(t *testing.T) { for i, tt := range tests { psName := fmt.Sprintf("test%d", i) - ps, err := conn.Prepare(psName, tt.sql) + ps, err := conn.Prepare(context.Background(), psName, tt.sql) if err != nil { t.Fatal(err) } diff --git a/pgtype/testutil/testutil.go b/pgtype/testutil/testutil.go index 3711381c..0d653394 100644 --- a/pgtype/testutil/testutil.go +++ b/pgtype/testutil/testutil.go @@ -107,7 +107,7 @@ func TestPgxSuccessfulTranscodeEqFunc(t testing.TB, pgTypeName string, values [] conn := MustConnectPgx(t) defer MustCloseContext(t, conn) - _, err := conn.Prepare("test", fmt.Sprintf("select $1::%s", pgTypeName)) + _, err := conn.Prepare(context.Background(), "test", fmt.Sprintf("select $1::%s", pgTypeName)) if err != nil { t.Fatal(err) } @@ -225,7 +225,7 @@ func TestPgxSuccessfulNormalizeEqFunc(t testing.TB, tests []NormalizeTest, eqFun for i, tt := range tests { for _, fc := range formats { psName := fmt.Sprintf("test%d", i) - ps, err := conn.Prepare(psName, tt.SQL) + ps, err := conn.Prepare(context.Background(), psName, tt.SQL) if err != nil { t.Fatal(err) } diff --git a/stdlib/sql.go b/stdlib/sql.go index cb4b5195..1bfbfcfc 100644 --- a/stdlib/sql.go +++ b/stdlib/sql.go @@ -162,7 +162,7 @@ func (c *Conn) PrepareContext(ctx context.Context, query string) (driver.Stmt, e name := fmt.Sprintf("pgx_%d", c.psCount) c.psCount++ - ps, err := c.conn.PrepareEx(ctx, name, query, nil) + ps, err := c.conn.Prepare(ctx, name, query) if err != nil { return nil, err } @@ -242,7 +242,7 @@ func (c *Conn) QueryContext(ctx context.Context, query string, argsV []driver.Na // TODO - remove hack that creates a new prepared statement for every query -- put in place because of problem preparing empty statement name psname := fmt.Sprintf("stdlibpx%v", &argsV) - ps, err := c.conn.PrepareEx(ctx, psname, query, nil) + ps, err := c.conn.Prepare(ctx, psname, query) if err != nil { // since PrepareEx failed, we didn't actually get to send the values, so // we can safely retry diff --git a/tx.go b/tx.go index 00a91d5d..bd0bd9f0 100644 --- a/tx.go +++ b/tx.go @@ -158,17 +158,12 @@ func (tx *Tx) Exec(ctx context.Context, sql string, arguments ...interface{}) (c } // Prepare delegates to the underlying *Conn -func (tx *Tx) Prepare(name, sql string) (*PreparedStatement, error) { - return tx.PrepareEx(context.Background(), name, sql, nil) -} - -// PrepareEx delegates to the underlying *Conn -func (tx *Tx) PrepareEx(ctx context.Context, name, sql string, opts *PrepareExOptions) (*PreparedStatement, error) { +func (tx *Tx) Prepare(ctx context.Context, name, sql string) (*PreparedStatement, error) { if tx.status != TxStatusInProgress { return nil, ErrTxClosed } - return tx.conn.PrepareEx(ctx, name, sql, opts) + return tx.conn.Prepare(ctx, name, sql) } // Query delegates to the underlying *Conn