diff --git a/README.md b/README.md index a4c66d6..ab78004 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ Firebird values, tracing hooks, and modern Firebird features. - Firebird `POST_EVENT` subscriptions through `fbevent.Manager`, including one-shot waits, callback listeners, channel subscriptions, and pgx-like notifications - Firebird 6 schema-qualified identifiers - Prepared statements, statement cache, configurable statement cache size, and configurable query execution mode -- Batch DML operations through object batches and named batches +- Batch DML operations through PreparedBatch or Exec with BatchRows - Positional `?` parameters plus `NamedArgs` query rewriting - Transactions with Firebird TPB support, isolation/access mode options, lock timeout, raw TPB mode, helper functions, and pseudo-nested transactions via savepoints - Streaming and lazy BLOB support, text and binary BLOB handling, configurable segment buffer size, stream BLOB mode, inline BLOB size, and inline BLOB cache settings diff --git a/batch.go b/batch.go index c6eef90..243d3f6 100644 --- a/batch.go +++ b/batch.go @@ -4,15 +4,74 @@ import ( "context" "errors" "fmt" - "strings" + "math" "github.com/firebirdsql/fbx/fbconn" + "github.com/firebirdsql/fbx/internal/operationerror" ) -const ( - batchPreparePrefix = "batch " - defaultPreparedBatchFlushSize = 128 * 1024 -) +// BatchRows contains parameter sets for one batch execution through [Conn.Exec] +// or [Tx.Exec]. Pass it as a single argument, without expanding it. Each row +// must contain one value per SQL parameter. An empty batch affects no rows. +// +// Exec uses the default [BatchOptions]. All encoded rows must fit in the server +// batch buffer; executions are not split into chunks. BLOB and ARRAY parameters +// are not supported. Use [PreparedBatch] for configurable, reusable batches. +type BatchRows [][]any + +func (c *Conn) execBatchRows(ctx context.Context, tx *dbTx, sql string, mode QueryExecMode, rows BatchRows) (status fbconn.QueryStatus, err error) { + if err := ctx.Err(); err != nil { + return status, err + } + b := PreparedBatch{conn: c, tx: tx, batchCfg: defaultPreparedBatchConfig()} + var temporary bool + // database/sql calls Conn.Exec even inside sql.Tx. Never finish that + // transaction, or an implicit transaction managed with Autocommit disabled. + releaseInternalTx := tx == nil && c.config.Autocommit && !c.fbConn.ActiveExplicitTx() + defer func() { + cleanupCtx, cancel := cleanupContext(ctx) + defer cancel() + cleanupErr := b.releaseWireResources(cleanupCtx) + if cleanupErr == nil && temporary { + cleanupErr = b.releaseStoredStatement(cleanupCtx) + } + // Prepare may have started a transaction even if encoding failed or + // the batch was empty. Execute already finishes it on its normal path. + if releaseInternalTx && !c.IsClosed() { + rbErr := c.Rollback(cleanupCtx) + if rbErr != nil && !errors.Is(rbErr, fbconn.ErrTransactionClosed) { + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("rollback internal tx: %w", rbErr)) + } + } + if cleanupErr != nil && !c.IsClosed() { + c.die() + } + err = operationerror.Join(err, cleanupErr) + if err != nil && b.st != nil && c.statementCache != nil { + c.statementCache.Invalidate(b.st.Description.SQL) + } + }() + + b.st, temporary, err = c.resolveStatement(ctx, tx, sql, mode) + if err != nil { + return status, err + } + if err := validateBatchStatement(b.st.Description); err != nil { + return status, err + } + statementType := b.st.Description.GetQueryType() + status = fbconn.NewQueryStatus(statementType, 0) + for i, row := range rows { + if err := ctx.Err(); err != nil { + return status, err + } + if err := b.Add(row...); err != nil { + return status, fmt.Errorf("batch row %d: %w", i, err) + } + } + result, err := b.Exec(ctx) + return fbconn.NewQueryStatus(statementType, result.Affected), err +} // BatchOptions configures batches created with [Conn.PrepareBatchWithOptions] // or [Tx.PrepareBatchWithOptions]. @@ -20,7 +79,8 @@ const ( // Zero values keep the driver defaults. type BatchOptions struct { // BufferBytes requests the server-side batch buffer size in bytes. - // Zero uses the server default. + // Zero uses the server default. Positive values up to math.MaxInt32 are + // accepted, but the server may cap the requested size. Negative values are invalid. BufferBytes int // ContinueOnError requests Firebird multi-error mode. When enabled, the @@ -39,26 +99,28 @@ type BatchOptions struct { // // Zero keeps the driver default: one detailed error in stop-on-first-error // mode, or the server default when ContinueOnError is enabled. + // Positive values up to math.MaxInt32 are accepted, but the server may cap + // the requested limit. Negative values are invalid. DetailedErrors int } -// BatchResult describes the server completion data returned by -// [PreparedBatch.Exec] and [Conn.ExecBatch]. +// BatchResult describes the server completion data returned by [PreparedBatch.Exec]. // // When execution returns a [BatchExecutionError], BatchResult still describes // what the server processed before the error response. In autocommit mode those // successful rows are rolled back by the driver when any per-row error is // reported. +// If execution fails without server completion data, the result is the zero value. type BatchResult struct { - // Affected is the sum of positive per-row update counts. If the server - // returned no per-row counts (RecordCounts disabled), Affected falls back - // to the server-reported total record count. + // Affected is the number of table rows affected by this execution, excluding + // failed batch elements. It is -1 when completion data does not provide all + // update counts, and 0 for an empty batch or when no rows were affected. Affected int64 // UpdateCounts holds per-row update counts in submission order, when - // RecordCounts is enabled (the default). Negative values are Firebird - // sentinels for failed/skipped rows; use BatchExecutionError.Errors for - // the cause. + // RecordCounts is enabled (the default). A value of -1 indicates a failed + // element (see BatchExecutionError.Errors); -2 indicates success without + // a known update count. // // Length: with BatchOptions.ContinueOnError there is one entry per // submitted row; in the default stop-on-first-error mode the slice may @@ -126,6 +188,9 @@ func (e *BatchExecutionError) Unwrap() []error { return errs } +// BatchInfo describes server batch buffers and the BLOB stream format. +type BatchInfo = fbconn.BatchInfo + // PreparedBatch stores encoded rows for one Firebird batch DML statement. // // Create a connection-scoped batch with [Conn.PrepareBatch] or @@ -144,6 +209,8 @@ func (e *BatchExecutionError) Unwrap() []error { // back the user's transaction; call [Tx.Commit] or [Tx.Rollback] yourself after // inspecting the result and error. // +// The statement, its metadata, and the server batch are retained until +// [PreparedBatch.Close], including between executions and after [PreparedBatch.Cancel]. // Always call [PreparedBatch.Close] when the batch is no longer needed. // [PreparedBatch.Cancel] can be used before [PreparedBatch.Exec] to discard both // local rows and rows already flushed to the server while keeping the @@ -153,23 +220,16 @@ type PreparedBatch struct { conn *Conn tx *dbTx - name string - sql string - - sd *fbconn.StatementDescription st *fbconn.Statement - activeSt *fbconn.Statement - releaseActiveSt bool - releaseActiveInternalTx bool - lb *fbconn.Batch + lb *fbconn.Batch - encodedRows [][]byte - pendingBytes int - autoFlushBytes int - batchCfg fbconn.BatchConfig - cleanupPending bool - closed bool + encodedRows [][]byte // Encoded rows waiting to be sent to the server. + pendingBytes int // Total bytes in encodedRows, excluding rows already sent. + batchCfg fbconn.BatchConfig // Options used whenever a server batch is created. + cleanupPending bool // Blocks use until Cancel confirms that the server queue is empty. + closing bool // Close has started; only another Close is allowed. + closed bool // Whether this PreparedBatch has been permanently closed. eqb ExtendedQueryBuilder } @@ -177,10 +237,40 @@ type PreparedBatch struct { // Add appends one encoded row to the local batch buffer. // // Add does not send batch data to the server and does not execute the batch. -// Object-style batches do not auto-flush; call [PreparedBatch.Flush] yourself -// when [PreparedBatch.PendingBytes] reaches a useful threshold. +// Call [PreparedBatch.Flush] when [PreparedBatch.PendingBytes] reaches a useful +// threshold, or [PreparedBatch.Exec] to send and execute all queued rows. func (b *PreparedBatch) Add(args ...any) error { - return b.addEncoded(context.Background(), args, false) + if err := b.checkReady(); err != nil { + return err + } + if b.st == nil || b.st.Description == nil { + return fmt.Errorf("prepared batch metadata is not initialized") + } + sd := b.st.Description + if len(args) != len(sd.Parameters) { + return fmt.Errorf("expected %d arguments, got %d", len(sd.Parameters), len(args)) + } + for i, param := range sd.Parameters { + if param.IsBlob() || isBlobArgument(args[i]) { + return fmt.Errorf("batch does not support blob parameters (arg %d)", i) + } + } + + if err := b.eqb.Build(context.Background(), b.conn, b.tx, b.conn.typeMap, sd, args); err != nil { + return err + } + + row, err := sd.EncodeBatchRow(b.eqb.ParamValues) + if err != nil { + return err + } + + b.eqb.reset() + + b.encodedRows = append(b.encodedRows, row) + b.pendingBytes += len(row) + + return nil } // PendingBytes returns the size of locally queued encoded rows that have not @@ -192,12 +282,22 @@ func (b *PreparedBatch) PendingBytes() int { return b.pendingBytes } -func isBatchPrepareSQL(sql string) (normalized string, ok bool) { - trimmed := strings.TrimSpace(sql) - if len(trimmed) < len(batchPreparePrefix) || !strings.EqualFold(trimmed[:len(batchPreparePrefix)], batchPreparePrefix) { - return "", false +// Info returns the current server buffer sizes and BLOB format parameters. +// It requires protocol version 17 or later (Firebird 4.0.1+). +// Info creates the server batch if necessary, but does not flush local rows, +// execute the batch, or finish a transaction. Local rows are counted separately +// by PendingBytes; their encoded size can differ from their server storage size. +func (b *PreparedBatch) Info(ctx context.Context) (BatchInfo, error) { + if err := b.checkReady(); err != nil { + return BatchInfo{}, err } - return strings.TrimSpace(trimmed[len(batchPreparePrefix):]), true + if b.st == nil { + return BatchInfo{}, fmt.Errorf("prepared batch statement is not initialized") + } + if b.lb == nil { + b.lb = b.st.NewBatch(b.batchCfg) + } + return b.lb.Info(ctx) } func validateBatchStatement(sd *fbconn.StatementDescription) error { @@ -235,7 +335,13 @@ func defaultPreparedBatchConfig() fbconn.BatchConfig { } } -func normalizeBatchOptions(opts BatchOptions) fbconn.BatchConfig { +func normalizeBatchOptions(opts BatchOptions) (fbconn.BatchConfig, error) { + if opts.BufferBytes < 0 || opts.BufferBytes > math.MaxInt32 { + return fbconn.BatchConfig{}, fmt.Errorf("batch buffer size must be between 0 and %d (got %d)", math.MaxInt32, opts.BufferBytes) + } + if opts.DetailedErrors < 0 || opts.DetailedErrors > math.MaxInt32 { + return fbconn.BatchConfig{}, fmt.Errorf("batch detailed error limit must be between 0 and %d (got %d)", math.MaxInt32, opts.DetailedErrors) + } cfg := defaultPreparedBatchConfig() if opts.BufferBytes > 0 { cfg.BufferBytes = opts.BufferBytes @@ -247,51 +353,35 @@ func normalizeBatchOptions(opts BatchOptions) fbconn.BatchConfig { if opts.DetailedErrors > 0 { cfg.DetailedErrs = opts.DetailedErrors } - return cfg + return cfg, nil } -func (b *PreparedBatch) addEncoded(ctx context.Context, args []any, allowAutoFlush bool) error { +func (b *PreparedBatch) checkOpen() error { if b == nil { return fmt.Errorf("prepared batch is nil") } if b.closed { return fmt.Errorf("prepared batch is closed") } - if b.cleanupPending { - return fmt.Errorf("prepared batch has pending cleanup after execution; close the batch before reusing it") - } - if b.sd == nil { - return fmt.Errorf("prepared batch metadata is not initialized") - } - if len(args) != len(b.sd.Parameters) { - return fmt.Errorf("expected %d arguments, got %d", len(b.sd.Parameters), len(args)) + if b.closing { + return fmt.Errorf("prepared batch is closing; retry Close to release resources") } - for i, param := range b.sd.Parameters { - if param.IsBlob() || isBlobArgument(args[i]) { - return fmt.Errorf("batch does not support blob parameters (arg %d)", i) - } + if b.conn != nil && b.conn.IsClosed() { + return fmt.Errorf("prepared batch connection is closed") } + return nil +} - if err := b.eqb.Build(ctx, b.conn, b.tx, b.conn.typeMap, b.sd, args); err != nil { +func (b *PreparedBatch) checkReady() error { + if err := b.checkOpen(); err != nil { return err } - - row, err := b.sd.EncodeBatchRow(b.eqb.ParamValues) - if err != nil { - return err + if b.cleanupPending { + return fmt.Errorf("prepared batch has pending cleanup; retry Cancel or Close") } - - b.eqb.reset() - - if allowAutoFlush && b.autoFlushBytes > 0 && b.pendingBytes > 0 && b.pendingBytes+len(row) > b.autoFlushBytes { - if err := b.flushWithSource(ctx, "auto"); err != nil { - return err - } + if b.tx != nil && b.tx.closed { + return ErrTxClosed } - - b.encodedRows = append(b.encodedRows, row) - b.pendingBytes += len(row) - return nil } @@ -301,34 +391,28 @@ func (b *PreparedBatch) addEncoded(ctx context.Context, args []any, allowAutoFlu // Rows sent by Flush stay pending on the server until [PreparedBatch.Exec], // [PreparedBatch.Cancel], or [PreparedBatch.Close]. Flush is a no-op when there // are no locally queued rows. +// +// An error before sending rows preserves the queue. If rows may have been sent, +// Flush discards the local queue and attempts to cancel all server rows using a +// separate, bounded context. Re-add the entire set after a successful reset; +// if reset fails, retry Cancel or Close before further use. func (b *PreparedBatch) Flush(ctx context.Context) error { - return b.flushWithSource(ctx, "manual") -} - -func (b *PreparedBatch) flushWithSource(ctx context.Context, source string) error { - if b == nil { - return fmt.Errorf("prepared batch is nil") - } - if b.closed { - return fmt.Errorf("prepared batch is closed") - } - if b.cleanupPending { - return fmt.Errorf("prepared batch has pending cleanup after execution; close the batch before reusing it") + if err := b.checkReady(); err != nil { + return err } if len(b.encodedRows) == 0 { return nil } - st, err := b.batchStatement(ctx) - if err != nil { - return err + if b.st == nil { + return fmt.Errorf("prepared batch statement is not initialized") } if b.lb == nil { - b.lb = st.NewBatch(b.batchCfg) + b.lb = b.st.NewBatch(b.batchCfg) } if err := b.lb.Send(ctx, b.encodedRows); err != nil { - return errors.Join(err, b.releaseWireResources(ctx)) + return b.resetAfterError(ctx, err) } b.ClearPending() return nil @@ -344,20 +428,17 @@ func (b *PreparedBatch) flushWithSource(ctx context.Context, source string) erro // error rolls back the internal transaction and no rows from that execution are // committed. In an explicit transaction, Exec leaves commit and rollback // decisions to the caller. +// +// A completed execution consumes all queued rows, including rows the server +// skipped after an error. The server batch remains open for the next set. +// An error before sending the execution command preserves the queue. An +// execution error without a completion attempts to reset it as described in +// [PreparedBatch.Flush]; it never repeats the execution automatically. func (b *PreparedBatch) Exec(ctx context.Context) (BatchResult, error) { - if b == nil { - return BatchResult{}, fmt.Errorf("prepared batch is nil") - } - if b.closed { - return BatchResult{}, fmt.Errorf("prepared batch is closed") - } - if b.cleanupPending { - return BatchResult{}, fmt.Errorf("prepared batch has pending cleanup after execution; close the batch before reusing it") - } - if err := b.flushWithSource(ctx, "exec-final"); err != nil { + if err := b.Flush(ctx); err != nil { return BatchResult{}, err } - if b.lb == nil { + if !b.lb.HasPending() { return BatchResult{}, nil } @@ -366,83 +447,24 @@ func (b *PreparedBatch) Exec(ctx context.Context) (BatchResult, error) { fbTx = b.tx.tx } completion, err := b.lb.Execute(ctx, fbTx) - b.cleanupPending = true - releaseErr := b.releaseWireResources(ctx) - return finalizePreparedBatchExec(completion, err, nil, releaseErr) -} - -func (c *Conn) shouldReleaseBatchInternalTx(tx *dbTx) bool { - return tx == nil && c.config.Autocommit && !c.fbConn.ActiveTx() -} - -func (c *Conn) rollbackBatchInternalTx(ctx context.Context, release bool) error { - if !release { - return nil - } - err := c.fbConn.Rollback(ctx) - if errors.Is(err, fbconn.ErrTransactionClosed) { - return nil - } - return err -} - -func (b *PreparedBatch) batchStatement(ctx context.Context) (*fbconn.Statement, error) { - if b.activeSt != nil { - return b.activeSt, nil - } - - if b.tx != nil { - if b.tx.closed { - return nil, ErrTxClosed - } - if b.st == nil { - return nil, fmt.Errorf("prepared batch statement is not initialized") - } - b.activeSt = b.st - b.releaseActiveSt = false - return b.activeSt, nil - } - - releaseInternalTx := b.conn.shouldReleaseBatchInternalTx(b.tx) - st, err := b.conn.internalPrepare(ctx, nil, "", b.sql) if err != nil { - return nil, errors.Join(err, b.conn.rollbackBatchInternalTx(ctx, releaseInternalTx)) + err = b.resetAfterError(ctx, err) } - b.activeSt = st - b.releaseActiveSt = true - b.releaseActiveInternalTx = releaseInternalTx - return st, nil + return finalizePreparedBatchExec(completion, err) } -func (b *PreparedBatch) releaseBatchStatement(ctx context.Context) error { - var err error - if b.activeSt != nil && b.releaseActiveSt { - if deallocErr := b.activeSt.Deallocate(ctx); deallocErr != nil { - err = errors.Join(err, deallocErr) - if b.conn == nil || b.conn.IsClosed() { - b.activeSt = nil - b.releaseActiveSt = false - } - } else { - b.activeSt = nil - b.releaseActiveSt = false - } - } else if b.activeSt != nil { - b.activeSt = nil +func (b *PreparedBatch) resetAfterError(ctx context.Context, operationErr error) error { + if !b.lb.NeedsReset() { + return operationErr } - - if b.releaseActiveInternalTx { - if rollbackErr := b.conn.rollbackBatchInternalTx(ctx, true); rollbackErr != nil { - err = errors.Join(err, rollbackErr) - if b.conn == nil || b.conn.IsClosed() { - b.releaseActiveInternalTx = false - } - } else { - b.releaseActiveInternalTx = false - } + b.cleanupPending = true + b.ClearPending() + if b.conn.IsClosed() { + return operationErr } - - return err + cleanupCtx, cancel := cleanupContext(ctx) + defer cancel() + return operationerror.Join(operationErr, b.Cancel(cleanupCtx)) } func (b *PreparedBatch) releaseWireResources(ctx context.Context) error { @@ -454,16 +476,14 @@ func (b *PreparedBatch) releaseWireResources(ctx context.Context) error { } b.lb = nil } - stErr := b.releaseBatchStatement(ctx) - err := errors.Join(batchErr, stErr) - if err == nil { + if batchErr == nil { b.cleanupPending = false } - return err + return batchErr } func (b *PreparedBatch) releaseStoredStatement(ctx context.Context) error { - if b.st == nil || b.activeSt == b.st { + if b.st == nil { return nil } err := b.st.Deallocate(ctx) @@ -489,38 +509,36 @@ func (b *PreparedBatch) ClearPending() { b.pendingBytes = 0 } -// Cancel performs a full reset: drops queued rows, releases the server-side -// batch context, and rolls back any internally owned statement and -// transaction. PreparedBatch stays valid — the next [PreparedBatch.Add], -// [PreparedBatch.Flush], or [PreparedBatch.Exec] re-prepares what it needs. For -// tx-scoped batches the user's statement and tx are left alone. +// Cancel drops both local and server rows, retaining the statement and server +// batch for reuse. Cancel does not finish any transaction or undo rows already +// applied by [PreparedBatch.Exec]. // -// Returns an error if the batch is closed or has pending cleanup from a prior -// [PreparedBatch.Exec] (call [PreparedBatch.Close] instead). On context -// cancellation, retry with a fresh context to free server-side resources. +// If cancellation is not confirmed, Add, Flush, Exec, and Info remain blocked until +// Cancel succeeds. Retry with a fresh context or call Close to release resources. func (b *PreparedBatch) Cancel(ctx context.Context) error { - if b == nil { - return fmt.Errorf("prepared batch is nil") - } - if b.closed { - return fmt.Errorf("prepared batch is closed") - } - if b.cleanupPending { - return fmt.Errorf("prepared batch has pending cleanup after execution; close the batch before reusing it") + if err := b.checkOpen(); err != nil { + return err } b.ClearPending() - return b.releaseWireResources(ctx) + b.cleanupPending = true + if err := b.lb.Cancel(ctx); err != nil { + return err + } + b.cleanupPending = false + return nil } // Close releases every resource owned by the batch and marks it unusable. // Idempotent. Must be called even after [PreparedBatch.Exec] returns an error, -// otherwise the prepared statement and any internal transaction stay allocated. -// On context cancellation, retry with a fresh context to free server-side -// resources. +// otherwise the prepared statement stays allocated. Close does not commit or +// roll back a user-managed transaction. +// Once Close starts, the batch cannot be reused. On context cancellation, retry +// Close with a fresh context to free the remaining server-side resources. func (b *PreparedBatch) Close(ctx context.Context) error { if b == nil || b.closed { return nil } + b.closing = true b.ClearPending() err := b.releaseWireResources(ctx) @@ -557,18 +575,18 @@ func batchCompletionError(completion *fbconn.BatchCompletion) error { return batchErr } -func finalizePreparedBatchExec(completion *fbconn.BatchCompletion, execErr, closeErr, releaseErr error) (BatchResult, error) { - if execErr != nil { - return BatchResult{}, errors.Join(execErr, closeErr, releaseErr) - } - +func finalizePreparedBatchExec(completion *fbconn.BatchCompletion, execErr error) (BatchResult, error) { result := BatchResult{} if completion != nil { + execErr = operationerror.WithRetry(execErr, false) result.Affected = completion.Affected() if len(completion.UpdateCounts) > 0 { result.UpdateCounts = append(make([]int64, 0, len(completion.UpdateCounts)), completion.UpdateCounts...) } } - return result, errors.Join(batchCompletionError(completion), closeErr, releaseErr) + if batchErr := batchCompletionError(completion); batchErr != nil { + execErr = operationerror.Join(batchErr, execErr) + } + return result, execErr } diff --git a/batch_insert.go b/batch_insert.go index 74a94b0..5b7ed11 100644 --- a/batch_insert.go +++ b/batch_insert.go @@ -3,10 +3,13 @@ package fbx import ( "bytes" "context" - "errors" "fmt" + + "github.com/firebirdsql/fbx/internal/operationerror" ) +const defaultBatchInsertPendingSize = 128 * 1024 + // BatchInsertRows adapts rows to the [BatchInsertRowsSource] interface used by // [Conn.BatchInsert]. func BatchInsertRows(rows [][]any) BatchInsertRowsSource { @@ -116,6 +119,13 @@ type batchInsert struct { } func (b *batchInsert) run(ctx context.Context) (affected int64, err error) { + if err := ctx.Err(); err != nil { + return 0, err + } + if b.conn.config.BatchInsertPendingSize < 0 { + return 0, fmt.Errorf("batch insert pending size must not be negative") + } + quotedTableName := b.tableName.Sanitize() cbuf := &bytes.Buffer{} pbuf := &bytes.Buffer{} @@ -130,19 +140,39 @@ func (b *batchInsert) run(ctx context.Context) (affected int64, err error) { quotedColumnNames := cbuf.String() placeholders := pbuf.String() - batch, err := b.conn.PrepareBatchWithOptions(ctx, fmt.Sprintf("insert into %s (%s) values (%s)", quotedTableName, quotedColumnNames, placeholders), BatchOptions{ContinueOnError: false}) + batch, err := b.conn.PrepareBatch(ctx, fmt.Sprintf("insert into %s (%s) values (%s)", quotedTableName, quotedColumnNames, placeholders)) if err != nil { return 0, err } + sourceRead := false defer func() { - if closeErr := batch.Close(ctx); closeErr != nil { - err = errors.Join(err, closeErr) + cleanupCtx, cancel := cleanupContext(ctx) + defer cancel() + cleanupErr := batch.Close(cleanupCtx) + if cleanupErr != nil && !b.conn.IsClosed() { + b.conn.die() + } + err = operationerror.Join(err, cleanupErr) + if sourceRead { + // Even a failed Next may advance the source. Repeating the call + // with the same source cannot replay the original rows reliably. + err = operationerror.WithRetry(err, false) } }() - for b.rowSrc.Next() { + for { + if err := ctx.Err(); err != nil { + return 0, err + } + sourceRead = true + if !b.rowSrc.Next() { + break + } + if err := ctx.Err(); err != nil { + return 0, err + } values, err := b.rowSrc.Values() - if err != nil { + if err := operationerror.Join(err, ctx.Err()); err != nil { return 0, err } err = batch.Add(values...) @@ -157,22 +187,30 @@ func (b *batchInsert) run(ctx context.Context) (affected int64, err error) { } } - if b.rowSrc.Err() != nil { - return 0, b.rowSrc.Err() - } - - br, err := batch.Exec(ctx) - if err != nil { + if err := operationerror.Join(b.rowSrc.Err(), ctx.Err()); err != nil { return 0, err } - return br.Affected, nil + br, err := batch.Exec(ctx) + return br.Affected, err } // BatchInsert uses Firebird native batch operations to perform bulk insertion. // -// BatchInsert returns the number of inserted rows. It uses quoted table and -// column identifiers, so identifiers are case-sensitive. Use upper-case +// BatchInsert sends rows as the source is read and executes them once, after +// the source is exhausted. All encoded rows must fit in the server batch buffer; +// flushing only limits client-side buffering and does not execute rows. +// +// The returned count comes from the server completion, or is zero if no +// completion was received. A count of -1 means the count is unknown. On error, +// even a positive count does not mean the rows were committed: in autocommit +// mode, a row error rolls back the entire execution. +// +// Errors after reading starts are not safe to retry with the same source. +// Context cancellation is checked between source calls; blocking source methods +// must handle context cancellation themselves. +// +// Table and column identifiers are quoted and case-sensitive. Use upper-case // identifiers for objects created with regular, unquoted SQL identifiers. // // See [BatchInsertRows], [BatchInsertFunc], and [BatchInsertSlice]. diff --git a/batch_insert_test.go b/batch_insert_test.go index 7594c1f..a2893a0 100644 --- a/batch_insert_test.go +++ b/batch_insert_test.go @@ -2,10 +2,8 @@ package fbx_test import ( "context" - "errors" "fmt" "math" - "math/rand" "os" "reflect" "testing" @@ -274,198 +272,32 @@ func (cfs *clientFailSource) Err() error { } func TestConnBatchInsertFailServerSideMidway(t *testing.T) { - t.Skip("need way to abort during execution") ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() config := mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE")) - config.BatchInsertPendingSize = 17 + config.BatchInsertPendingSize = 0 // Flush each row, execute only at the end. conn := mustConnect(t, config) defer closeConn(t, conn) skipIfBatchUnsupported(ctx, t, conn) - mustExec(t, conn, `recreate table foo( - a int, - b varchar(10) not null - )`) - - inputRows := [][]any{ - {int32(1), "abc"}, - {int32(2), nil}, - {int32(3), "def"}, - } - count, err := conn.BatchInsert(ctx, fbx.Identifier{"FOO"}, []string{"A", "B"}, fbx.BatchInsertRows(inputRows)) - if err == nil { - t.Errorf("Expected BatchInsert return error, but it did not") - } - var batchExecutionError *fbconn.FbError - if !errors.As(err, &batchExecutionError) { - t.Errorf("Expected BatchInsert return fbconn.FbError, but instead it returned: %v", err) - } - if count != 0 { - t.Errorf("Expected BatchInsert to return 0 inserted rows, but got %d", count) - } - - rows, err := conn.Query(ctx, "select * from foo") - if err != nil { - t.Errorf("Unexpected error for Query: %v", err) - } - - var outputRows [][]any - for rows.Next() { - row, err := rows.Values() - if err != nil { - t.Errorf("Unexpected error for rows.Values(): %v", err) - } - outputRows = append(outputRows, row) - } - - if rows.Err() != nil { - t.Errorf("Unexpected error for rows.Err(): %v", rows.Err()) - } - - if len(outputRows) != 0 { - t.Errorf("Expected 0 rows, but got %v", outputRows) - } - - ensureConnValid(t, conn) -} - -type failSource struct { - count int -} - -func (fs *failSource) Next() bool { - time.Sleep(time.Millisecond * 100) - fs.count++ - return fs.count < 100 -} - -const letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" - -func randomString(n int) string { - b := make([]byte, n) - for i := range b { - b[i] = letters[rand.Intn(len(letters))] - } - return string(b) -} - -func (fs *failSource) Values() ([]any, error) { - if fs.count == 1280 { - return []any{nil}, nil - } - return []any{randomString(1024)}, nil -} - -func (fs *failSource) Err() error { - return nil -} - -func TestConnBatchInsertFailServerSideMidwayAbortsWithoutWaiting(t *testing.T) { - t.Skip("need way to abort during execution") - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - conn := mustConnectString(t, os.Getenv("FBX_TEST_DATABASE")) - defer closeConn(t, conn) - skipIfBatchUnsupported(ctx, t, conn) - - mustExec(t, conn, `recreate table foo( - a varchar(1024) not null - )`) - - startTime := time.Now() - - count, err := conn.BatchInsert(ctx, fbx.Identifier{"FOO"}, []string{"A"}, &failSource{}) - if err == nil { - t.Errorf("Expected BatchInsert return error, but it did not") - } - var fbError *fbconn.FbError - if !errors.As(err, &fbError) { - t.Errorf("Expected BatchInsert return fbconn.FbError, but instead it returned: %v", err) - } - if count != 0 { - t.Errorf("Expected BatchInsert to return 0 inserted rows, but got %d", count) - } - - endTime := time.Now() - batchTime := endTime.Sub(startTime) - if batchTime > time.Second { - t.Errorf("Failing BatchInsert shouldn't have taken so long: %v", batchTime) - } - - rows, err := conn.Query(ctx, "select * from foo") - if err != nil { - t.Errorf("Unexpected error for Query: %v", err) - } - - var outputRows [][]any - for rows.Next() { - row, err := rows.Values() - if err != nil { - t.Errorf("Unexpected error for rows.Values(): %v", err) - } - outputRows = append(outputRows, row) - } - - if rows.Err() != nil { - t.Errorf("Unexpected error for rows.Err(): %v", rows.Err()) - } - - if len(outputRows) != 0 { - t.Errorf("Expected 0 rows, but got %v", outputRows) - } - - ensureConnValid(t, conn) -} - -type slowFailRaceSource struct { - count int -} - -func (fs *slowFailRaceSource) Next() bool { - time.Sleep(time.Millisecond) - fs.count++ - return fs.count < 1000 -} - -func (fs *slowFailRaceSource) Values() ([]any, error) { - if fs.count == 500 { - return []any{nil, nil}, nil - } - return []any{1, "1234567890"}, nil -} - -func (fs *slowFailRaceSource) Err() error { - return nil -} - -func TestConnBatchInsertSlowFailRace(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - conn := mustConnectString(t, os.Getenv("FBX_TEST_DATABASE")) - defer closeConn(t, conn) - skipIfBatchUnsupported(ctx, t, conn) - - mustExec(t, conn, `recreate table foo( - a int not null, - b varchar(10) not null - )`) - - count, err := conn.BatchInsert(ctx, fbx.Identifier{"FOO"}, []string{"A", "B"}, &slowFailRaceSource{}) - if err == nil { - t.Errorf("Expected BatchInsert return error, but it did not") - } - var fbError *fbconn.FbError - if !errors.As(err, &fbError) { - t.Errorf("Expected BatchInsert return fbconn.FbError, but instead it returned: %v", err) - } - if count != 0 { - t.Errorf("Expected BatchInsert to return 0 inserted rows, but got %d", count) - } - + mustExec(t, conn, "recreate table batch_insert_failure (id integer not null)") + inputRows := [][]any{{int32(1)}, {nil}, {int32(3)}} + read := 0 + source := fbx.BatchInsertSlice(len(inputRows), func(i int) ([]any, error) { + read++ + return inputRows[i], nil + }) + count, err := conn.BatchInsert(ctx, fbx.Identifier{"BATCH_INSERT_FAILURE"}, []string{"ID"}, source) + var batchErr *fbx.BatchExecutionError + require.ErrorAs(t, err, &batchErr) + require.False(t, fbconn.SafeToRetry(err)) + require.Equal(t, len(inputRows), read) + require.EqualValues(t, 1, count) // Completed before the error, then rolled back. + + var stored int + require.NoError(t, conn.QueryRow(ctx, "select count(*) from batch_insert_failure").Scan(&stored)) + require.Zero(t, stored) ensureConnValid(t, conn) } @@ -852,24 +684,10 @@ func ExampleBatchInsertSlice() { return } - dataCh := make(chan []any, 3) - - const channelItems = 3 - go func() { - for i := range channelItems { + count, err := conn.BatchInsert(ctx, fbx.Identifier{"FOO"}, []string{"A", "B", "C"}, + fbx.BatchInsertSlice(3, func(i int) ([]any, error) { n := i + 1 - dataCh <- []any{n, n + 1, n + 2} - } - close(dataCh) - }() - - count, err := conn.BatchInsert(context.Background(), fbx.Identifier{"FOO"}, []string{"A", "B", "C"}, - fbx.BatchInsertFunc(func() ([]any, error) { - v, ok := <-dataCh - if !ok { - return nil, nil - } - return v, nil + return []any{n, n + 1, n + 2}, nil })) if err != nil { fmt.Println(err) diff --git a/batch_internal_test.go b/batch_internal_test.go index 92571f4..2040ae6 100644 --- a/batch_internal_test.go +++ b/batch_internal_test.go @@ -3,15 +3,15 @@ package fbx import ( "context" "errors" + "math" "os" - "reflect" - "sync/atomic" + "strconv" "testing" - "unsafe" "github.com/stretchr/testify/require" "github.com/firebirdsql/fbx/fbconn" + "github.com/firebirdsql/fbx/internal/operationerror" ) func skipIfBatchUnsupported(ctx context.Context, t testing.TB, conn *Conn) { @@ -22,46 +22,282 @@ func skipIfBatchUnsupported(ctx context.Context, t testing.TB, conn *Conn) { } } -func forceStatementActiveBatch(t testing.TB, st *fbconn.Statement) { - t.Helper() - require.NotNil(t, st) +func TestPreparedBatchInfo(t *testing.T) { + for _, scope := range []string{"connection", "transaction"} { + t.Run(scope, func(t *testing.T) { + ctx := context.Background() + conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) + defer func() { require.NoError(t, conn.Close(ctx)) }() + if !conn.ServerVersion().AtLeast(4, 0, 1) { + t.Skip("batch info requires Firebird 4.0.1 or later") + } + _, err := conn.Exec(ctx, "recreate table batch_info_test (id integer)") + require.NoError(t, err) + prepare := conn.PrepareBatchWithOptions + var tx Tx + if scope == "transaction" { + tx, err = conn.Begin(ctx) + require.NoError(t, err) + prepare = tx.PrepareBatchWithOptions + } + b, err := prepare(ctx, "insert into batch_info_test values (?)", BatchOptions{BufferBytes: 65536}) + require.NoError(t, err) + defer func() { require.NoError(t, b.Close(ctx)) }() + require.NoError(t, b.Add(1)) + pending := b.PendingBytes() + info, err := b.Info(ctx) + require.NoError(t, err) + require.Equal(t, 65536, info.BufferBytes) + require.Zero(t, info.DataBytes) + require.Zero(t, info.BlobBytes) + require.Positive(t, info.BlobAlignment) + require.Positive(t, info.BlobHeaderBytes) + require.Equal(t, pending, b.PendingBytes(), "Info must not flush local rows") + require.Equal(t, tx != nil, conn.fbConn.ActiveTx()) + + require.NoError(t, b.Flush(ctx)) + info, err = b.Info(ctx) + require.NoError(t, err) + require.Positive(t, info.DataBytes) + require.Zero(t, b.PendingBytes()) + require.Equal(t, tx != nil, conn.fbConn.ActiveTx()) + require.NoError(t, b.Add(2)) + before, err := b.Info(ctx) + require.NoError(t, err) + require.Equal(t, info, before) + require.Equal(t, pending, b.PendingBytes()) + canceledCtx, cancel := context.WithCancel(ctx) + cancel() + _, err = b.Info(canceledCtx) + require.ErrorIs(t, err, context.Canceled) + require.Equal(t, pending, b.PendingBytes()) + + result, err := b.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 2, result.Affected) + info, err = b.Info(ctx) + require.NoError(t, err) + require.Zero(t, info.DataBytes) + require.Equal(t, tx != nil, conn.fbConn.ActiveTx(), "Info must not finish a transaction") + require.NoError(t, b.Add(3)) + require.NoError(t, b.Flush(ctx)) + require.NoError(t, b.Cancel(ctx)) + info, err = b.Info(ctx) + require.NoError(t, err) + require.Zero(t, info.DataBytes) + require.NoError(t, b.Close(ctx)) + _, err = b.Info(ctx) + require.Error(t, err) + + want := 2 + if tx != nil { + require.NoError(t, tx.Rollback(ctx)) + want = 0 + } + var count int + require.NoError(t, conn.QueryRow(ctx, "select count(*) from batch_info_test").Scan(&count)) + require.Equal(t, want, count) + }) + } +} + +type cancelAfterBatchPrepare struct { + cancel context.CancelFunc +} + +func (tracer cancelAfterBatchPrepare) TracePrepareStart(ctx context.Context, _ *Conn, _ TracePrepareStartData) context.Context { + return ctx +} - field := reflect.ValueOf(st).Elem().FieldByName("activeBatch") - require.True(t, field.IsValid()) +func (tracer cancelAfterBatchPrepare) TracePrepareEnd(context.Context, *Conn, TracePrepareEndData) { + tracer.cancel() +} - reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).Elem().Set(reflect.ValueOf(&fbconn.Batch{})) +func TestExecBatchRowsCanceledAfterPrepare(t *testing.T) { + for _, mode := range []QueryExecMode{QueryExecModeCacheStatement, QueryExecModeExec} { + t.Run(mode.String(), func(t *testing.T) { + ctx := context.Background() + config := mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE")) + config.DefaultQueryExecMode = mode + conn := mustConnect(t, config) + defer func() { require.NoError(t, conn.Close(ctx)) }() + skipIfBatchUnsupported(ctx, t, conn) + _, err := conn.Exec(ctx, "recreate table exec_batch_cancel_test (id integer)") + require.NoError(t, err) + + canceledCtx, cancel := context.WithCancel(ctx) + defer cancel() + conn.prepareTracer = cancelAfterBatchPrepare{cancel: cancel} + _, err = conn.Exec(canceledCtx, "insert into exec_batch_cancel_test values (?)", BatchRows{{1}}) + require.ErrorIs(t, err, context.Canceled) + require.False(t, conn.IsClosed()) + require.False(t, conn.fbConn.ActiveTx()) + }) + } } -type cancelOnDoneCallContext struct { - context.Context +func TestBatchInsertRejectsBeforePrepare(t *testing.T) { + conn := &Conn{config: &ConnConfig{BatchInsertPendingSize: -1}} + source := BatchInsertFunc(func() ([]any, error) { + t.Fatal("source must not be read") + return nil, nil + }) + _, err := conn.BatchInsert(context.Background(), Identifier{"T"}, []string{"ID"}, source) + require.ErrorContains(t, err, "pending size must not be negative") - cancelOnCall int32 - doneCalls atomic.Int32 - closed chan struct{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err = conn.BatchInsert(ctx, Identifier{"T"}, []string{"ID"}, source) + require.ErrorIs(t, err, context.Canceled) } -func newCancelOnDoneCallContext(cancelOnCall int32) *cancelOnDoneCallContext { - closed := make(chan struct{}) - close(closed) - return &cancelOnDoneCallContext{ - Context: context.Background(), - cancelOnCall: cancelOnCall, - closed: closed, +func TestBatchInsertSourceErrors(t *testing.T) { + for _, stage := range []string{"before Next", "Next", "Values", "end of source", "first Next error"} { + t.Run(stage, func(t *testing.T) { + config := mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE")) + config.BatchInsertPendingSize = 0 + conn := mustConnect(t, config) + defer func() { require.NoError(t, conn.Close(context.Background())) }() + skipIfBatchUnsupported(context.Background(), t, conn) + _, err := conn.Exec(context.Background(), "recreate table batch_insert_source (id integer)") + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + sourceErr := operationerror.WithRetry(errors.New("source failed"), true) + calls := 0 + next := func() ([]any, error) { + calls++ + if stage == "first Next error" { + return nil, sourceErr + } + if calls == 1 { + return []any{1}, nil + } + cancel() + if stage == "end of source" { + return nil, sourceErr + } + return []any{2}, nil + } + source := BatchInsertFunc(next) + if stage == "Values" { + source = BatchInsertSlice(3, func(int) ([]any, error) { return next() }) + } + if stage == "before Next" { + conn.prepareTracer = cancelAfterBatchPrepare{cancel: cancel} + } + count, err := conn.BatchInsert(ctx, Identifier{"BATCH_INSERT_SOURCE"}, []string{"ID"}, source) + if stage == "first Next error" { + require.ErrorIs(t, err, sourceErr) + require.Equal(t, 1, calls) + } else { + require.ErrorIs(t, err, context.Canceled) + if stage == "before Next" { + require.Zero(t, calls) + } else { + require.Equal(t, 2, calls) + } + } + if stage == "end of source" { + require.ErrorIs(t, err, sourceErr) + } + require.Zero(t, count) + require.False(t, fbconn.SafeToRetry(err)) + require.False(t, conn.IsClosed()) + require.False(t, conn.fbConn.ActiveTx()) + conn.prepareTracer = nil + require.Zero(t, countStatementHandlesForSQL(t, conn, `insert into "BATCH_INSERT_SOURCE" ("ID") values (?)`)) + var stored int + require.NoError(t, conn.QueryRow(context.Background(), "select count(*) from batch_insert_source").Scan(&stored)) + require.Zero(t, stored) + }) } } -func (ctx *cancelOnDoneCallContext) Done() <-chan struct{} { - if ctx.doneCalls.Add(1) >= ctx.cancelOnCall { - return ctx.closed +func TestBatchInsertCleanupFailureClosesConnection(t *testing.T) { + ctx := context.Background() + config := mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE")) + config.BatchInsertPendingSize = 0 + conn := mustConnect(t, config) + defer func() { require.NoError(t, conn.Close(ctx)) }() + skipIfBatchUnsupported(ctx, t, conn) + _, err := conn.Exec(ctx, "recreate table batch_insert_cleanup (id integer)") + require.NoError(t, err) + + sourceErr := operationerror.WithRetry(errors.New("source failed"), true) + var rows Rows + defer func() { + if rows != nil { + rows.Close() + } + }() + source := BatchInsertSlice(2, func(i int) ([]any, error) { + if i == 0 { + return []any{1}, nil + } + // Leave a result open so cleanup cannot acquire the connection. + rows, err = conn.Query(ctx, "select 1 from rdb$database") + require.NoError(t, err) + return nil, sourceErr + }) + count, err := conn.BatchInsert(ctx, Identifier{"BATCH_INSERT_CLEANUP"}, []string{"ID"}, source) + require.Zero(t, count) + require.ErrorIs(t, err, sourceErr) + require.ErrorContains(t, err, "conn busy") + require.False(t, fbconn.SafeToRetry(err)) + require.True(t, conn.IsClosed()) +} + +func TestPrepareBatchRejectsInvalidOptionsBeforePrepare(t *testing.T) { + type testCase struct { + name string + opts BatchOptions + err string + } + tests := []testCase{ + {"negative buffer", BatchOptions{BufferBytes: -1}, "batch buffer size"}, + {"negative details", BatchOptions{DetailedErrors: -1}, "batch detailed error limit"}, + } + if strconv.IntSize == 64 { + overflow := int64(math.MaxInt32) + 1 + tests = append(tests, + testCase{"buffer overflow", BatchOptions{BufferBytes: int(overflow)}, "batch buffer size"}, + testCase{"details overflow", BatchOptions{DetailedErrors: int(overflow)}, "batch detailed error limit"}, + ) + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + conn := &Conn{} + tx := &dbTx{conn: conn} + for _, prepare := range []func(context.Context, string, BatchOptions) (*PreparedBatch, error){ + conn.PrepareBatchWithOptions, tx.PrepareBatchWithOptions, + } { + batch, err := prepare(context.Background(), "insert into t values (?)", tt.opts) + require.ErrorContains(t, err, tt.err) + require.Nil(t, batch) + } + }) } - return nil } -func (ctx *cancelOnDoneCallContext) Err() error { - if ctx.doneCalls.Load() >= ctx.cancelOnCall { - return context.Canceled +func TestNormalizeBatchOptions(t *testing.T) { + tests := []struct { + name string + opts BatchOptions + want fbconn.BatchConfig + }{ + {"defaults", BatchOptions{}, fbconn.BatchConfig{RecordCounts: true, BufferBytes: -1, DetailedErrs: 1}}, + {"multi-error defaults", BatchOptions{ContinueOnError: true}, fbconn.BatchConfig{MultiError: true, RecordCounts: true, BufferBytes: -1, DetailedErrs: -1}}, + {"maximum wire values", BatchOptions{BufferBytes: math.MaxInt32, DetailedErrors: math.MaxInt32, ContinueOnError: true}, fbconn.BatchConfig{MultiError: true, RecordCounts: true, BufferBytes: math.MaxInt32, DetailedErrs: math.MaxInt32}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg, err := normalizeBatchOptions(tt.opts) + require.NoError(t, err) + require.Equal(t, tt.want, cfg) + }) } - return nil } func TestPrepareBatchWithOptionsPropagatesConfig(t *testing.T) { @@ -144,6 +380,7 @@ func TestPrepareBatchConnectionScopeClosesMetadataTransaction(t *testing.T) { err := conn.Close(ctx) require.NoError(t, err) }() + skipIfBatchUnsupported(ctx, t, conn) table := "batch_internal_tx_meta" _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") @@ -154,8 +391,39 @@ func TestPrepareBatchConnectionScopeClosesMetadataTransaction(t *testing.T) { require.NoError(t, err) require.False(t, conn.fbConn.ActiveTx()) + st := pb.st + require.NotNil(t, st) + var serverBatch *fbconn.Batch + for _, id := range []int{1, 2} { + require.NoError(t, pb.Add(id, "saved")) + require.NoError(t, pb.Flush(ctx)) + if serverBatch == nil { + serverBatch = pb.lb + } + require.True(t, pb.lb.HasPending()) + require.False(t, conn.fbConn.ActiveTx(), "Flush must not open a transaction") + result, err := pb.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, result.Affected) + require.Same(t, st, pb.st, "Exec must retain the original statement") + require.Same(t, serverBatch, pb.lb, "Exec must retain the server batch") + require.False(t, pb.lb.HasPending()) + require.False(t, conn.fbConn.ActiveTx()) + } + + require.NoError(t, pb.Add(3, "discarded")) + require.NoError(t, pb.Flush(ctx)) + require.NoError(t, pb.Cancel(ctx)) + require.Same(t, st, pb.st, "Cancel must retain the original statement") + require.Same(t, serverBatch, pb.lb, "Cancel must retain the server batch") + require.False(t, pb.lb.HasPending()) + require.NoError(t, pb.Close(ctx)) + require.Nil(t, pb.st) require.False(t, conn.fbConn.ActiveTx()) + var count int + require.NoError(t, conn.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Equal(t, 2, count) } func TestPrepareBatchRejectClosesMetadataTransaction(t *testing.T) { @@ -167,12 +435,37 @@ func TestPrepareBatchRejectClosesMetadataTransaction(t *testing.T) { require.NoError(t, err) }() - _, err := conn.PrepareBatch(ctx, "select cast(? as integer) from rdb$database") + for _, sql := range []string{"select cast(? as integer) from rdb$database", "invalid batch SQL"} { + prepareCtx, cancel := context.WithCancel(ctx) + conn.prepareTracer = cancelAfterBatchPrepare{cancel: cancel} + _, err := conn.PrepareBatch(prepareCtx, sql) + cancel() + require.Error(t, err) + require.NotErrorIs(t, err, context.Canceled, "cleanup must preserve the preparation or validation error") + require.False(t, conn.fbConn.ActiveTx()) + require.False(t, conn.IsClosed(), "cleanup after cancellation must preserve a healthy connection") + } +} + +func TestPrepareBatchClosesMetadataTransactionWithAnotherTxActive(t *testing.T) { + ctx := context.Background() + conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) + defer func() { require.NoError(t, conn.Close(ctx)) }() + + firstTx, err := conn.Begin(ctx) + require.NoError(t, err) + lastTx, err := conn.Begin(ctx) + require.NoError(t, err) + require.NoError(t, lastTx.Rollback(ctx)) + + _, err = conn.PrepareBatch(ctx, "select cast(? as integer) from rdb$database") require.Error(t, err) - require.False(t, conn.fbConn.ActiveTx()) + require.True(t, conn.fbConn.ActiveTx(), "the first user transaction must remain active") + require.NoError(t, firstTx.Rollback(ctx)) + require.False(t, conn.fbConn.ActiveTx(), "preparation must not leave its own transaction behind") } -func TestPreparedBatchFlushCloseReleasesOwnedInternalTransaction(t *testing.T) { +func TestPreparedBatchClosePreservesLaterTransaction(t *testing.T) { ctx := context.Background() conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) @@ -191,10 +484,56 @@ func TestPreparedBatchFlushCloseReleasesOwnedInternalTransaction(t *testing.T) { require.NoError(t, err) require.NoError(t, pb.Add(1, "pending")) require.NoError(t, pb.Flush(ctx)) - require.True(t, conn.fbConn.ActiveTx()) + require.False(t, conn.fbConn.ActiveTx()) + tx, err := conn.Begin(ctx) + require.NoError(t, err) + _, err = tx.Exec(ctx, "insert into "+table+" (id, note) values (2, 'user tx')") + require.NoError(t, err) + require.NoError(t, pb.Close(ctx)) + require.True(t, conn.fbConn.ActiveTx(), "Close must not finish the user's transaction") + var count int + require.NoError(t, tx.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Equal(t, 1, count) + require.NoError(t, tx.Rollback(ctx)) + require.False(t, conn.fbConn.ActiveTx(), "the batch must not leave an old metadata transaction behind") + require.NoError(t, conn.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Zero(t, count) +} + +func TestPreparedBatchPreservesImplicitTransaction(t *testing.T) { + ctx := context.Background() + config := mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE")) + config.Autocommit = false + conn := mustConnect(t, config) + defer func() { require.NoError(t, conn.Close(ctx)) }() + skipIfBatchUnsupported(ctx, t, conn) + + table := "batch_manual_commit" + _, err := conn.Exec(ctx, "recreate table "+table+" (id integer)") + require.NoError(t, err) + require.NoError(t, conn.Commit(ctx)) + _, err = conn.Exec(ctx, "insert into "+table+" values (1)") + require.NoError(t, err) + + pb, err := conn.PrepareBatch(ctx, "insert into "+table+" values (?)") + require.NoError(t, err) + require.NoError(t, pb.Add(2)) + result, err := pb.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, result.Affected) require.NoError(t, pb.Close(ctx)) + require.True(t, conn.fbConn.ActiveTx()) + _, err = conn.Exec(ctx, "insert into "+table+" values (?)", BatchRows{{3}}) + require.NoError(t, err) + require.True(t, conn.fbConn.ActiveTx(), "BatchRows must preserve the manually managed transaction") + var count int + require.NoError(t, conn.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Equal(t, 3, count, "both batch APIs must preserve earlier uncommitted work") + require.NoError(t, conn.Rollback(ctx)) require.False(t, conn.fbConn.ActiveTx()) + require.NoError(t, conn.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Zero(t, count, "Exec must not commit when Autocommit is disabled") } func TestPreparedBatchCloseCanceledContextCanRetry(t *testing.T) { @@ -215,7 +554,7 @@ func TestPreparedBatchCloseCanceledContextCanRetry(t *testing.T) { require.NoError(t, err) require.NoError(t, pb.Add(1, "pending")) require.NoError(t, pb.Flush(ctx)) - require.True(t, conn.fbConn.ActiveTx()) + require.False(t, conn.fbConn.ActiveTx()) canceledCtx, cancel := context.WithCancel(ctx) cancel() @@ -224,7 +563,12 @@ func TestPreparedBatchCloseCanceledContextCanRetry(t *testing.T) { require.Error(t, err) require.False(t, pb.closed) require.NotNil(t, pb.lb) - require.True(t, conn.fbConn.ActiveTx()) + require.False(t, conn.fbConn.ActiveTx()) + require.ErrorContains(t, pb.Add(2, "blocked"), "closing") + require.ErrorContains(t, pb.Flush(ctx), "closing") + _, err = pb.Exec(ctx) + require.ErrorContains(t, err, "closing") + require.ErrorContains(t, pb.Cancel(ctx), "closing") require.NoError(t, pb.Close(ctx)) require.True(t, pb.closed) @@ -232,7 +576,7 @@ func TestPreparedBatchCloseCanceledContextCanRetry(t *testing.T) { require.False(t, conn.fbConn.ActiveTx()) } -func TestPreparedBatchExecCanceledCleanupCanRetry(t *testing.T) { +func TestPreparedBatchExecCanceledBeforeSendingCanRetry(t *testing.T) { ctx := context.Background() conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) @@ -250,7 +594,7 @@ func TestPreparedBatchExecCanceledCleanupCanRetry(t *testing.T) { require.NoError(t, err) require.NoError(t, pb.Add(1, "pending")) require.NoError(t, pb.Flush(ctx)) - require.True(t, conn.fbConn.ActiveTx()) + require.False(t, conn.fbConn.ActiveTx()) canceledCtx, cancel := context.WithCancel(ctx) cancel() @@ -259,7 +603,12 @@ func TestPreparedBatchExecCanceledCleanupCanRetry(t *testing.T) { require.Error(t, err) require.False(t, pb.closed) require.NotNil(t, pb.lb) - require.True(t, conn.fbConn.ActiveTx()) + require.False(t, conn.fbConn.ActiveTx(), "failed execution must release the owned internal transaction") + require.True(t, pb.lb.HasPending()) + require.False(t, pb.lb.NeedsReset()) + result, err := pb.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, result.Affected) require.NoError(t, pb.Close(ctx)) require.True(t, pb.closed) @@ -267,7 +616,7 @@ func TestPreparedBatchExecCanceledCleanupCanRetry(t *testing.T) { require.False(t, conn.fbConn.ActiveTx()) } -func TestPreparedBatchExecCanceledCleanupDoesNotReexecute(t *testing.T) { +func TestPreparedBatchEmptyExecDoesNotReexecute(t *testing.T) { ctx := context.Background() conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) @@ -292,15 +641,17 @@ func TestPreparedBatchExecCanceledCleanupDoesNotReexecute(t *testing.T) { require.NoError(t, pb.Add(1)) require.NoError(t, pb.Flush(ctx)) - cleanupCanceledCtx := newCancelOnDoneCallContext(3) - result, err := pb.Exec(cleanupCanceledCtx) - require.ErrorIs(t, err, context.Canceled) + result, err := pb.Exec(ctx) + require.NoError(t, err) require.EqualValues(t, 1, result.Affected) require.NotNil(t, pb.lb) - result, err = pb.Exec(ctx) - require.Error(t, err, "Exec after a successful execute with pending cleanup must not reexecute the previous batch") + canceledCtx, cancel := context.WithCancel(ctx) + cancel() + result, err = pb.Exec(canceledCtx) + require.NoError(t, err, "empty Exec must not start a transaction or send a command") require.Zero(t, result.Affected) + require.False(t, conn.fbConn.ActiveTx()) require.NoError(t, pb.Close(ctx)) @@ -332,7 +683,7 @@ func TestPreparedBatchFlushCanceledCanRetry(t *testing.T) { require.NoError(t, pb.Add(1, "first")) require.NoError(t, pb.Flush(ctx)) - require.True(t, conn.fbConn.ActiveTx()) + require.False(t, conn.fbConn.ActiveTx()) require.NoError(t, pb.Add(2, "second")) canceledCtx, cancel := context.WithCancel(ctx) @@ -342,7 +693,7 @@ func TestPreparedBatchFlushCanceledCanRetry(t *testing.T) { require.ErrorIs(t, err, context.Canceled) require.NotNil(t, pb.lb) require.NotZero(t, pb.PendingBytes()) - require.True(t, conn.fbConn.ActiveTx()) + require.False(t, conn.fbConn.ActiveTx()) require.NoError(t, pb.Flush(ctx)) require.Zero(t, pb.PendingBytes()) @@ -395,7 +746,7 @@ func TestTxPreparedBatchCloseCanceledStatementCanRetry(t *testing.T) { require.Nil(t, pb.st) } -func TestTxCommitCanceledKeepsNamedBatchForRetry(t *testing.T) { +func TestTxCommitCanceledPreservesPreparedBatch(t *testing.T) { ctx := context.Background() conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) @@ -406,7 +757,6 @@ func TestTxCommitCanceledKeepsNamedBatchForRetry(t *testing.T) { skipIfBatchUnsupported(ctx, t, conn) table := "batch_tx_commit_retry" - name := "batch_tx_commit_retry_named" _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") require.NoError(t, err) @@ -421,12 +771,11 @@ func TestTxCommitCanceledKeepsNamedBatchForRetry(t *testing.T) { } }() - _, err = tx.Prepare(ctx, name, "batch insert into "+table+" (id, note) values (?, ?)") - require.NoError(t, err) - _, err = tx.Exec(ctx, name, 1, "pending") + pb, err := tx.PrepareBatch(ctx, "insert into "+table+" (id, note) values (?, ?)") require.NoError(t, err) - require.NoError(t, tx.Flush(ctx, name)) - require.Contains(t, conn.preparedBatches, name) + defer func() { require.NoError(t, pb.Close(ctx)) }() + require.NoError(t, pb.Add(1, "pending")) + require.NoError(t, pb.Flush(ctx)) require.True(t, conn.fbConn.ActiveTx()) canceledCtx, cancel := context.WithCancel(ctx) @@ -435,40 +784,24 @@ func TestTxCommitCanceledKeepsNamedBatchForRetry(t *testing.T) { err = tx.Commit(canceledCtx) require.ErrorIs(t, err, context.Canceled) require.False(t, tx.closed) - require.Contains(t, conn.preparedBatches, name) + require.False(t, pb.closing) + require.True(t, pb.lb.HasPending()) require.True(t, conn.fbConn.ActiveTx()) + result, err := pb.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, result.Affected) require.NoError(t, tx.Commit(ctx)) require.True(t, tx.closed) - require.NotContains(t, conn.preparedBatches, name) - require.False(t, conn.fbConn.ActiveTx()) -} - -func TestNamedBatchFlushDeallocateReleasesOwnedInternalTransaction(t *testing.T) { - ctx := context.Background() - - conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) - defer func() { - err := conn.Close(ctx) - require.NoError(t, err) - }() - skipIfBatchUnsupported(ctx, t, conn) - - table := "batch_internal_tx_flush_named" - name := "batch_internal_tx_named" - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - require.False(t, conn.fbConn.ActiveTx()) - - _, err = conn.Prepare(ctx, name, "batch insert into "+table+" (id, note) values (?, ?)") - require.NoError(t, err) - _, err = conn.Exec(ctx, name, 1, "pending") - require.NoError(t, err) - require.NoError(t, conn.Flush(ctx, name)) - require.True(t, conn.fbConn.ActiveTx()) - - require.NoError(t, conn.Deallocate(ctx, name)) require.False(t, conn.fbConn.ActiveTx()) + require.ErrorIs(t, pb.Add(2, "after commit"), ErrTxClosed) + require.ErrorIs(t, pb.Flush(ctx), ErrTxClosed) + _, err = pb.Exec(ctx) + require.ErrorIs(t, err, ErrTxClosed) + require.NoError(t, pb.Close(ctx)) + var count int + require.NoError(t, conn.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Equal(t, 1, count) } func TestPreparedBatchClearPendingClearsBufferedRows(t *testing.T) { @@ -543,10 +876,15 @@ func TestPreparedBatchCancelGuards(t *testing.T) { require.EqualError(t, pb.Cancel(ctx), "prepared batch is closed") }) - t.Run("cleanup pending refuses cancel", func(t *testing.T) { + t.Run("closing batch refuses cancel", func(t *testing.T) { + pb := &PreparedBatch{closing: true} + require.ErrorContains(t, pb.Cancel(ctx), "prepared batch is closing") + }) + + t.Run("cancel completes pending cleanup", func(t *testing.T) { pb := &PreparedBatch{cleanupPending: true} - require.ErrorContains(t, pb.Cancel(ctx), - "prepared batch has pending cleanup after execution") + require.NoError(t, pb.Cancel(ctx)) + require.False(t, pb.cleanupPending) }) t.Run("no wire state — Cancel clears local buffer and returns nil", func(t *testing.T) { @@ -565,38 +903,65 @@ func TestPreparedBatchCancelGuards(t *testing.T) { } func TestFinalizePreparedBatchExecJoinsErrors(t *testing.T) { + t.Run("unknown affected count is not an execution error", func(t *testing.T) { + completion := &fbconn.BatchCompletion{TotalRecords: 2} + + result, err := finalizePreparedBatchExec(completion, nil) + + require.NoError(t, err) + require.EqualValues(t, -1, result.Affected) + require.Nil(t, result.UpdateCounts) + }) + + t.Run("completion survives cancellation and retryable cleanup", func(t *testing.T) { + rowErr := errors.New("row cancelled") + completion := &fbconn.BatchCompletion{ + TotalRecords: 2, + UpdateCounts: []int64{1, -1}, + Errors: []fbconn.BatchError{{Row: 1, Err: rowErr}}, + } + cleanupErr := operationerror.WithRetry(errors.New("cleanup failed"), true) + result, err := finalizePreparedBatchExec(completion, operationerror.Join(context.DeadlineExceeded, cleanupErr)) + + require.EqualValues(t, 1, result.Affected) + require.Equal(t, completion.UpdateCounts, result.UpdateCounts) + require.ErrorIs(t, err, context.DeadlineExceeded) + require.ErrorIs(t, err, rowErr) + require.ErrorIs(t, err, cleanupErr) + require.False(t, fbconn.SafeToRetry(err)) + var batchErr *BatchExecutionError + require.ErrorAs(t, err, &batchErr) + }) + t.Run("execute error keeps cleanup errors", func(t *testing.T) { execErr := errors.New("execute failed") - closeErr := errors.New("close failed") - releaseErr := errors.New("release failed") + cleanupErr := errors.New("cancel failed") - result, err := finalizePreparedBatchExec(nil, execErr, closeErr, releaseErr) + result, err := finalizePreparedBatchExec(nil, operationerror.Join(execErr, cleanupErr)) require.Zero(t, result.Affected) require.ErrorContains(t, err, "execute failed") - require.ErrorContains(t, err, "close failed") - require.ErrorContains(t, err, "release failed") + require.ErrorIs(t, err, cleanupErr) }) t.Run("batch row error keeps affected and cleanup errors", func(t *testing.T) { - closeErr := errors.New("close failed") - releaseErr := errors.New("release failed") + rollbackErr := errors.New("rollback failed") completion := &fbconn.BatchCompletion{ - UpdateCounts: []int64{1}, + TotalRecords: 3, + UpdateCounts: []int64{1, 0, -1}, Errors: []fbconn.BatchError{ {Row: 2, Err: errors.New("duplicate key")}, }, } - result, err := finalizePreparedBatchExec(completion, nil, closeErr, releaseErr) + result, err := finalizePreparedBatchExec(completion, rollbackErr) require.EqualValues(t, 1, result.Affected) - require.Equal(t, []int64{1}, result.UpdateCounts) + require.Equal(t, completion.UpdateCounts, result.UpdateCounts) require.ErrorContains(t, err, "batch failed at row 2") require.ErrorContains(t, err, "duplicate key") - require.ErrorContains(t, err, "close failed") - require.ErrorContains(t, err, "release failed") + require.ErrorIs(t, err, rollbackErr) var batchErr *BatchExecutionError require.ErrorAs(t, err, &batchErr) @@ -605,7 +970,8 @@ func TestFinalizePreparedBatchExecJoinsErrors(t *testing.T) { t.Run("multiple batch row errors are exposed", func(t *testing.T) { completion := &fbconn.BatchCompletion{ - UpdateCounts: []int64{1, -1, 1, -1}, + TotalRecords: 6, + UpdateCounts: []int64{1, -1, 1, -1, 0, -1}, Errors: []fbconn.BatchError{ {Row: 1, Err: errors.New("first duplicate")}, {Row: 3, Err: errors.New("second duplicate")}, @@ -613,10 +979,10 @@ func TestFinalizePreparedBatchExecJoinsErrors(t *testing.T) { }, } - result, err := finalizePreparedBatchExec(completion, nil, nil, nil) + result, err := finalizePreparedBatchExec(completion, nil) require.EqualValues(t, 2, result.Affected) - require.Equal(t, []int64{1, -1, 1, -1}, result.UpdateCounts) + require.Equal(t, completion.UpdateCounts, result.UpdateCounts) require.ErrorContains(t, err, "batch failed at row 1") require.ErrorContains(t, err, "2 more batch errors") @@ -632,103 +998,3 @@ func TestFinalizePreparedBatchExecJoinsErrors(t *testing.T) { "simplified errors must wrap ErrBatchRowFailed so errors.Is can see them") }) } - -func TestTxCommitDoesNotStopOnNamedBatchCloseError(t *testing.T) { - ctx := context.Background() - - conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) - defer func() { - err := conn.Close(ctx) - require.NoError(t, err) - }() - skipIfBatchUnsupported(ctx, t, conn) - - checkConn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) - defer func() { - err := checkConn.Close(ctx) - require.NoError(t, err) - }() - - table := "tx_commit_named_batch_close_err_test" - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - - txi, err := conn.Begin(ctx) - require.NoError(t, err) - tx, ok := txi.(*dbTx) - require.True(t, ok) - - _, err = tx.Exec(ctx, "insert into "+table+"(id, note) values (?, ?)", 1, "committed") - require.NoError(t, err) - - _, err = tx.Prepare(ctx, "commit_close_err_batch", "batch update "+table+" set note = ? where id = ?") - require.NoError(t, err) - - pb := conn.preparedBatches["commit_close_err_batch"] - require.NotNil(t, pb) - require.NotNil(t, pb.st) - - forceStatementActiveBatch(t, pb.st) - - err = tx.Commit(ctx) - require.Error(t, err) - require.ErrorContains(t, err, "statement has active batch") - require.True(t, tx.closed) - require.NotContains(t, conn.preparedBatches, "commit_close_err_batch") - - var note string - err = checkConn.QueryRow(ctx, "select note from "+table+" where id = ?", 1).Scan(¬e) - require.NoError(t, err) - require.Equal(t, "committed", note) -} - -func TestTxRollbackDoesNotStopOnNamedBatchCloseError(t *testing.T) { - ctx := context.Background() - - conn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) - defer func() { - err := conn.Close(ctx) - require.NoError(t, err) - }() - skipIfBatchUnsupported(ctx, t, conn) - - checkConn := mustConnect(t, mustParseConfig(t, os.Getenv("FBX_TEST_DATABASE"))) - defer func() { - err := checkConn.Close(ctx) - require.NoError(t, err) - }() - - table := "tx_rollback_named_batch_close_err_test" - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - - txi, err := conn.Begin(ctx) - require.NoError(t, err) - tx, ok := txi.(*dbTx) - require.True(t, ok) - - _, err = tx.Exec(ctx, "insert into "+table+"(id, note) values (?, ?)", 1, "rolled back") - require.NoError(t, err) - - _, err = tx.Prepare(ctx, "rollback_close_err_batch", "batch update "+table+" set note = ? where id = ?") - require.NoError(t, err) - - pb := conn.preparedBatches["rollback_close_err_batch"] - require.NotNil(t, pb) - require.NotNil(t, pb.st) - - forceStatementActiveBatch(t, pb.st) - - err = tx.Rollback(ctx) - require.Error(t, err) - require.ErrorContains(t, err, "statement has active batch") - require.True(t, tx.closed) - require.NotContains(t, conn.preparedBatches, "rollback_close_err_batch") - - var count int - err = checkConn.QueryRow(ctx, "select count(*) from "+table).Scan(&count) - require.NoError(t, err) - require.Zero(t, count) -} diff --git a/batch_test.go b/batch_test.go index bd6f6b1..016437c 100644 --- a/batch_test.go +++ b/batch_test.go @@ -7,9 +7,74 @@ import ( "time" "github.com/firebirdsql/fbx" + "github.com/firebirdsql/fbx/fbconn" + "github.com/firebirdsql/fbx/fbxtest" "github.com/stretchr/testify/require" ) +func TestExecBatchRows(t *testing.T) { + ctx := context.Background() + fbxtest.RunWithQueryExecModes(ctx, t, defaultConnTestRunner, nil, func(ctx context.Context, t testing.TB, conn *fbx.Conn) { + skipIfBatchUnsupported(ctx, t, conn) + table := batchTestTableName(t, "fbx_exec_rows") + _, err := conn.Exec(ctx, "recreate table "+table+" (id integer primary key, note varchar(30))") + require.NoError(t, err) + query := "insert into " + table + " (id, note) values (?, ?)" + + status, err := conn.Exec(ctx, query, fbx.BatchRows{{1, "one"}, {2, "two"}}) + require.NoError(t, err) + require.True(t, status.Insert()) + require.EqualValues(t, 2, status.RowsAffected()) + require.False(t, conn.FbConn().ActiveTx()) + + _, err = conn.Exec(ctx, query, 3, "ordinary") + require.NoError(t, err) + _, err = conn.Prepare(ctx, "exec_rows", query) + require.NoError(t, err) + status, err = conn.Exec(ctx, "exec_rows", fbx.BatchRows{{4, "named"}}) + require.NoError(t, err) + require.EqualValues(t, 1, status.RowsAffected()) + require.NoError(t, conn.Deallocate(ctx, "exec_rows")) + + status, err = conn.Exec(ctx, query, fbx.BatchRows(nil)) + require.NoError(t, err) + require.Zero(t, status.RowsAffected()) + require.False(t, conn.FbConn().ActiveTx()) + _, err = conn.Exec(ctx, query, fbx.BatchRows{{5, "valid"}, {6}}) + require.ErrorContains(t, err, "batch row 1") + require.False(t, conn.FbConn().ActiveTx()) + + _, err = conn.Exec(ctx, query, fbx.BatchRows{{5, "rolled back"}, {1, "duplicate"}}) + var batchErr *fbx.BatchExecutionError + require.ErrorAs(t, err, &batchErr) + require.Equal(t, 1, batchErr.Errors[0].Row) + require.False(t, fbconn.SafeToRetry(err)) + require.False(t, conn.FbConn().ActiveTx()) + var count int + require.NoError(t, conn.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Equal(t, 4, count) + + _, err = conn.Query(ctx, query, fbx.BatchRows{{5, "invalid API"}}) + require.ErrorContains(t, err, "BatchRows must be executed with Exec") + require.False(t, conn.FbConn().ActiveTx()) + + tx, err := conn.Begin(ctx) + require.NoError(t, err) + defer func() { _ = tx.Rollback(ctx) }() + _, err = tx.Exec(ctx, query, fbx.BatchRows{{5, "first"}}) + require.NoError(t, err) + _, err = tx.Exec(ctx, query, fbx.BatchRows{{6, "second"}}) + require.NoError(t, err) + _, err = tx.Exec(ctx, query, fbx.BatchRows{{1, "duplicate"}}) + require.ErrorAs(t, err, &batchErr) + require.NoError(t, tx.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Equal(t, 6, count) + require.NoError(t, tx.Rollback(ctx)) + require.NoError(t, conn.QueryRow(ctx, "select count(*) from "+table).Scan(&count)) + require.Equal(t, 4, count) + }) +} + func TestTxPrepareBatchUpdateDeleteCommit(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() @@ -203,10 +268,7 @@ func TestTxBatchMethodsReturnErrTxClosed(t *testing.T) { _, err = tx.PrepareBatch(ctx, "insert into rdb$database(dummy) values (?)") require.ErrorIs(t, err, fbx.ErrTxClosed) - err = tx.Flush(ctx, "missing") - require.ErrorIs(t, err, fbx.ErrTxClosed) - - _, err = tx.ExecBatch(ctx, "missing") + _, err = tx.PrepareBatchWithOptions(ctx, "insert into rdb$database(dummy) values (?)", fbx.BatchOptions{}) require.ErrorIs(t, err, fbx.ErrTxClosed) } @@ -379,7 +441,14 @@ func TestPrepareBatchWithOptionsBufferBytesOverflow(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() - conn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) + requestCtx, cancelRequest := context.WithCancel(ctx) + defer cancelRequest() + config := defaultConnTestRunner.CreateConfig(ctx, t) + config.OnFbError = func(_ *fbconn.FbConn, _ *fbconn.FbError) bool { + cancelRequest() + return true + } + conn := mustConnect(t, config) defer closeConn(t, conn) skipIfBatchUnsupported(ctx, t, conn) @@ -388,7 +457,7 @@ func TestPrepareBatchWithOptionsBufferBytesOverflow(t *testing.T) { table := batchTestTableName(t, "fbx_pb_opts_overflow") - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(4096))") + _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(512))") require.NoError(t, err) pb, err := conn.PrepareBatchWithOptions(ctx, "insert into "+table+" (id, note) values (?, ?)", fbx.BatchOptions{ @@ -399,16 +468,29 @@ func TestPrepareBatchWithOptionsBufferBytesOverflow(t *testing.T) { require.NoError(t, pb.Close(ctx)) }() - require.NoError(t, pb.Add(1, strings.Repeat("a", 2048))) - require.NoError(t, pb.Add(2, strings.Repeat("b", 2048))) - require.NoError(t, pb.Add(3, strings.Repeat("c", 2048))) + require.NoError(t, pb.Add(0, "already flushed")) + require.NoError(t, pb.Flush(ctx)) + for id := 1; id <= 32; id++ { + require.NoError(t, pb.Add(id, strings.Repeat("a", 256))) + } require.Greater(t, pb.PendingBytes(), 0) - result, err := pb.Exec(ctx) + result, err := pb.Exec(requestCtx) require.Error(t, err) + require.ErrorIs(t, err, context.Canceled) + require.False(t, fbconn.SafeToRetry(err)) require.EqualValues(t, 0, result.Affected) require.ErrorContains(t, err, "Internal buffer overflow") + require.Zero(t, pb.PendingBytes()) require.Empty(t, loadBatchDMLRows(t, ctx, inspect, table)) + result, err = pb.Exec(ctx) + require.NoError(t, err) + require.Zero(t, result.Affected, "reset must also discard rows from earlier Flush calls") + require.NoError(t, pb.Add(10, "after reset")) + result, err = pb.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, result.Affected) + require.Equal(t, []batchDMLRow{{ID: 10, Note: "after reset"}}, loadBatchDMLRows(t, ctx, inspect, table)) } func TestTxPrepareBatchWithOptionsBufferBytesCommit(t *testing.T) { @@ -469,67 +551,6 @@ func TestTxPrepareBatchClosedTxErrorType(t *testing.T) { require.ErrorIs(t, err, fbx.ErrTxClosed) } -func TestTxPrepareNamedBatchUpdateDeleteCommit(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - conn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) - defer closeConn(t, conn) - skipIfBatchUnsupported(ctx, t, conn) - - checkConn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) - defer closeConn(t, checkConn) - - table := batchTestTableName(t, "fbx_txnb_commit") - updateName := batchTestTableName(t, "upd_batch") - deleteName := batchTestTableName(t, "del_batch") - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - seedBatchDMLRows(t, ctx, conn, table) - - tx, err := conn.Begin(ctx) - require.NoError(t, err) - - _, err = tx.Prepare(ctx, updateName, "batch update "+table+" set note = ? where id = ?") - require.NoError(t, err) - - _, err = tx.Exec(ctx, updateName, "one-tx-named", 1) - require.NoError(t, err) - _, err = tx.Exec(ctx, updateName, "two-tx-named", 2) - require.NoError(t, err) - - updateResult, err := tx.ExecBatch(ctx, updateName) - require.NoError(t, err) - require.EqualValues(t, 2, updateResult.Affected) - - _, err = tx.Prepare(ctx, deleteName, "batch delete from "+table+" where id = ?") - require.NoError(t, err) - - _, err = tx.Exec(ctx, deleteName, 3) - require.NoError(t, err) - _, err = tx.Exec(ctx, deleteName, 4) - require.NoError(t, err) - - deleteResult, err := tx.ExecBatch(ctx, deleteName) - require.NoError(t, err) - require.EqualValues(t, 2, deleteResult.Affected) - - require.Equal(t, []batchDMLRow{ - {ID: 1, Note: "one"}, - {ID: 2, Note: "two"}, - {ID: 3, Note: "three"}, - {ID: 4, Note: "four"}, - }, loadBatchDMLRows(t, ctx, checkConn, table)) - - require.NoError(t, tx.Commit(ctx)) - - require.Equal(t, []batchDMLRow{ - {ID: 1, Note: "one-tx-named"}, - {ID: 2, Note: "two-tx-named"}, - }, loadBatchDMLRows(t, ctx, checkConn, table)) -} - func TestPreparedBatchReuseUpdateDelete(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() @@ -582,98 +603,6 @@ func TestPreparedBatchReuseUpdateDelete(t *testing.T) { }, loadBatchDMLRows(t, ctx, conn, table)) } -func TestNamedBatchReuseUpdateDelete(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - conn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) - defer closeConn(t, conn) - skipIfBatchUnsupported(ctx, t, conn) - - table := batchTestTableName(t, "fbx_nb_reuse_ud") - updateName := batchTestTableName(t, "upd_batch") - deleteName := batchTestTableName(t, "del_batch") - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - seedBatchDMLRows(t, ctx, conn, table) - - _, err = conn.Prepare(ctx, updateName, "batch update "+table+" set note = ? where id = ?") - require.NoError(t, err) - defer func() { require.NoError(t, conn.Deallocate(ctx, updateName)) }() - - _, err = conn.Exec(ctx, updateName, "one-named-reuse", 1) - require.NoError(t, err) - updateResult, err := conn.ExecBatch(ctx, updateName) - require.NoError(t, err) - require.EqualValues(t, 1, updateResult.Affected) - - _, err = conn.Exec(ctx, updateName, "two-named-reuse", 2) - require.NoError(t, err) - updateResult, err = conn.ExecBatch(ctx, updateName) - require.NoError(t, err) - require.EqualValues(t, 1, updateResult.Affected) - - _, err = conn.Prepare(ctx, deleteName, "batch delete from "+table+" where id = ?") - require.NoError(t, err) - defer func() { require.NoError(t, conn.Deallocate(ctx, deleteName)) }() - - _, err = conn.Exec(ctx, deleteName, 3) - require.NoError(t, err) - deleteResult, err := conn.ExecBatch(ctx, deleteName) - require.NoError(t, err) - require.EqualValues(t, 1, deleteResult.Affected) - - _, err = conn.Exec(ctx, deleteName, 4) - require.NoError(t, err) - deleteResult, err = conn.ExecBatch(ctx, deleteName) - require.NoError(t, err) - require.EqualValues(t, 1, deleteResult.Affected) - - require.Equal(t, []batchDMLRow{ - {ID: 1, Note: "one-named-reuse"}, - {ID: 2, Note: "two-named-reuse"}, - }, loadBatchDMLRows(t, ctx, conn, table)) -} - -func TestNamedBatchExecConsumesOptionsAndQueryRewriter(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - conn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) - defer closeConn(t, conn) - skipIfBatchUnsupported(ctx, t, conn) - - table := batchTestTableName(t, "fbx_nb_options") - name := batchTestTableName(t, "batch_options") - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - - _, err = conn.Prepare(ctx, name, "batch insert into "+table+" (id, note) values (?, ?)") - require.NoError(t, err) - defer func() { require.NoError(t, conn.Deallocate(ctx, name)) }() - - _, err = conn.Exec(ctx, name, fbx.QueryExecModeExec, 1, "one") - require.NoError(t, err) - - qr := testQueryRewriter{ - sql: name, - args: []any{2, "two"}, - } - _, err = conn.Exec(ctx, "rewritten_batch_name", &qr) - require.NoError(t, err) - - result, err := conn.ExecBatch(ctx, name) - require.NoError(t, err) - require.EqualValues(t, 2, result.Affected) - - require.Equal(t, []batchDMLRow{ - {ID: 1, Note: "one"}, - {ID: 2, Note: "two"}, - }, loadBatchDMLRows(t, ctx, conn, table)) -} - func TestPreparedBatchFlushUpdateDelete(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() @@ -1005,6 +934,13 @@ func TestPreparedBatchCancelDiscardsFlushedRowsAndAllowsReuse(t *testing.T) { require.NoError(t, pb.Add(2, "second")) require.NoError(t, pb.Flush(ctx)) + canceledCtx, cancelRequest := context.WithCancel(ctx) + cancelRequest() + require.ErrorIs(t, pb.Cancel(canceledCtx), context.Canceled) + require.ErrorContains(t, pb.Add(3, "blocked"), "pending cleanup") + require.ErrorContains(t, pb.Flush(ctx), "pending cleanup") + _, err = pb.Exec(ctx) + require.ErrorContains(t, err, "pending cleanup") require.NoError(t, pb.Cancel(ctx)) // Exec after Cancel sees no queued rows and is a no-op. @@ -1054,48 +990,7 @@ func TestPreparedBatchCancelWithoutFlushOnlyClearsLocalQueue(t *testing.T) { require.Empty(t, loadBatchDMLRows(t, ctx, conn, table)) } -func TestConnCancelBatchDiscardsFlushedNamedRowsAndAllowsReuse(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - conn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) - defer closeConn(t, conn) - skipIfBatchUnsupported(ctx, t, conn) - - table := batchTestTableName(t, "fbx_nb_cancel") - batchName := batchTestTableName(t, "ins_batch") - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - - _, err = conn.Prepare(ctx, batchName, "batch insert into "+table+" (id, note) values (?, ?)") - require.NoError(t, err) - defer func() { require.NoError(t, conn.Deallocate(ctx, batchName)) }() - - _, err = conn.Exec(ctx, batchName, 1, "first") - require.NoError(t, err) - _, err = conn.Exec(ctx, batchName, 2, "second") - require.NoError(t, err) - require.NoError(t, conn.Flush(ctx, batchName)) - - require.NoError(t, conn.CancelBatch(ctx, batchName)) - - result, err := conn.ExecBatch(ctx, batchName) - require.NoError(t, err) - require.EqualValues(t, 0, result.Affected) - require.Empty(t, loadBatchDMLRows(t, ctx, conn, table)) - - _, err = conn.Exec(ctx, batchName, 10, "post-cancel") - require.NoError(t, err) - result, err = conn.ExecBatch(ctx, batchName) - require.NoError(t, err) - require.EqualValues(t, 1, result.Affected) - require.Equal(t, []batchDMLRow{ - {ID: 10, Note: "post-cancel"}, - }, loadBatchDMLRows(t, ctx, conn, table)) -} - -func TestConnCancelBatchUnknownNameReturnsError(t *testing.T) { +func TestTxPreparedBatchCancelDiscardsFlushedRows(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() @@ -1103,42 +998,28 @@ func TestConnCancelBatchUnknownNameReturnsError(t *testing.T) { defer closeConn(t, conn) skipIfBatchUnsupported(ctx, t, conn) - err := conn.CancelBatch(ctx, "does-not-exist") - require.ErrorContains(t, err, `prepared batch "does-not-exist" not found`) -} - -func TestTxCancelBatchDiscardsFlushedRows(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - conn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) - defer closeConn(t, conn) - skipIfBatchUnsupported(ctx, t, conn) - - table := batchTestTableName(t, "fbx_txnb_cancel") - batchName := batchTestTableName(t, "ins_batch") + table := batchTestTableName(t, "fbx_txpb_cancel") _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") require.NoError(t, err) tx, err := conn.Begin(ctx) require.NoError(t, err) + defer func() { _ = tx.Rollback(ctx) }() - _, err = tx.Prepare(ctx, batchName, "batch insert into "+table+" (id, note) values (?, ?)") + pb, err := tx.PrepareBatch(ctx, "insert into "+table+" (id, note) values (?, ?)") require.NoError(t, err) + defer func() { require.NoError(t, pb.Close(ctx)) }() - _, err = tx.Exec(ctx, batchName, 1, "first") - require.NoError(t, err) - _, err = tx.Exec(ctx, batchName, 2, "second") - require.NoError(t, err) - require.NoError(t, tx.Flush(ctx, batchName)) + require.NoError(t, pb.Add(1, "first")) + require.NoError(t, pb.Add(2, "second")) + require.NoError(t, pb.Flush(ctx)) - require.NoError(t, tx.CancelBatch(ctx, batchName)) + require.NoError(t, pb.Cancel(ctx)) // After cancel within tx, a new row can be added and executed. - _, err = tx.Exec(ctx, batchName, 10, "post-cancel") - require.NoError(t, err) - result, err := tx.ExecBatch(ctx, batchName) + require.NoError(t, pb.Add(10, "post-cancel")) + result, err := pb.Exec(ctx) require.NoError(t, err) require.EqualValues(t, 1, result.Affected) @@ -1201,6 +1082,7 @@ func TestPreparedBatchAutocommitRollsBackOnRowError(t *testing.T) { require.NoError(t, pb.Add(1, "one")) require.NoError(t, pb.Add(2, nil)) + require.NoError(t, pb.Add(3, "skipped after error")) _, err = pb.Exec(ctx) @@ -1209,39 +1091,66 @@ func TestPreparedBatchAutocommitRollsBackOnRowError(t *testing.T) { require.Empty(t, loadBatchDMLRows(t, ctx, checkConn, table), "valid first row must not be committed when a later row fails") + result, err := pb.Exec(ctx) + require.NoError(t, err) + require.Zero(t, result.Affected, "a completion must consume skipped rows too") + require.NoError(t, pb.Add(4, "after error")) + result, err = pb.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, result.Affected) + require.Equal(t, []batchDMLRow{{ID: 4, Note: "after error"}}, loadBatchDMLRows(t, ctx, checkConn, table)) } -func TestNamedBatchAutocommitRollsBackOnRowError(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) +func TestPreparedBatchCanceledWhileLocked(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - - conn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) + dsn := defaultConnTestRunner.CreateConfig(ctx, t).ConnString() + conn := mustConnectString(t, dsn) defer closeConn(t, conn) skipIfBatchUnsupported(ctx, t, conn) - - checkConn := mustConnectString(t, defaultConnTestRunner.CreateConfig(ctx, t).ConnString()) - defer closeConn(t, checkConn) - - table := batchTestTableName(t, "fbx_nb_partial") - batchName := batchTestTableName(t, "ins_batch") - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50) not null)") + locker := mustConnectString(t, dsn) + defer closeConn(t, locker) + table := batchTestTableName(t, "fbx_pb_cancel") + _, err := locker.Exec(ctx, "recreate table "+table+" (id integer primary key, v integer)") require.NoError(t, err) - - _, err = conn.Prepare(ctx, batchName, "batch insert into "+table+" (id, note) values (?, ?)") + defer func() { + require.NoError(t, conn.Close(ctx)) + require.NoError(t, locker.DeallocateAll(ctx)) + _, err := locker.Exec(ctx, "drop table "+table) + require.NoError(t, err) + }() + for _, id := range []int{1, 2} { + _, err = locker.Exec(ctx, "insert into "+table+" values (?, 0)", id) + require.NoError(t, err) + } + tx, err := locker.Begin(ctx) require.NoError(t, err) - defer func() { require.NoError(t, conn.Deallocate(ctx, batchName)) }() - - _, err = conn.Exec(ctx, batchName, 1, "one") + defer func() { _ = tx.Rollback(ctx) }() + _, err = tx.Exec(ctx, "update "+table+" set v = 99 where id = 2") require.NoError(t, err) - _, err = conn.Exec(ctx, batchName, 2, nil) + pb, err := conn.PrepareBatch(ctx, "update "+table+" set v = ? where id = ?") require.NoError(t, err) + defer func() { require.NoError(t, pb.Close(ctx)) }() + require.NoError(t, pb.Add(10, 1)) + require.NoError(t, pb.Add(20, 2)) + requestCtx, cancelRequest := context.WithTimeout(ctx, 300*time.Millisecond) + defer cancelRequest() - _, err = conn.ExecBatch(ctx, batchName) + result, err := pb.Exec(requestCtx) + require.ErrorIs(t, err, context.DeadlineExceeded) + require.False(t, fbconn.SafeToRetry(err)) var batchErr *fbx.BatchExecutionError require.ErrorAs(t, err, &batchErr) - - require.Empty(t, loadBatchDMLRows(t, ctx, checkConn, table), - "valid first row must not be committed when a later row fails") + require.EqualValues(t, 1, result.Affected) + require.NoError(t, tx.Rollback(ctx)) + require.False(t, conn.IsClosed(), "successful server cancellation must preserve the connection") + require.False(t, conn.FbConn().ActiveTx()) + var sum int + require.NoError(t, conn.QueryRow(ctx, "select sum(v) from "+table).Scan(&sum)) + require.Zero(t, sum, "the next query must not commit the first batch row") + require.NoError(t, pb.Add(30, 1)) + result, err = pb.Exec(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, result.Affected, "batch remains usable after confirmed server cancellation") } diff --git a/conn.go b/conn.go index a4190d6..ee21eb2 100644 --- a/conn.go +++ b/conn.go @@ -35,8 +35,9 @@ type ConnConfig struct { // may cache statement/description metadata to reduce round trips and parsing overhead. DefaultQueryExecMode QueryExecMode - // BatchInsertPendingSize is maximum size of pending data in batch before - // [Conn.BatchInsert] will flush it to server. + // BatchInsertPendingSize is the client buffer threshold in bytes for + // [Conn.BatchInsert]. A row may exceed it. Zero flushes every row; negative + // values are invalid. Flushing does not execute or empty the server batch. BatchInsertPendingSize int createdByParseConfig bool // Used to enforce created by ParseConfig rule. @@ -68,7 +69,6 @@ type Conn struct { fbConn *fbconn.FbConn config *ConnConfig // config used when establishing this connection preparedStatements map[string]*fbconn.Statement - preparedBatches map[string]*PreparedBatch statementCache stmtcache.Cache queryTracer QueryTracer @@ -280,13 +280,16 @@ func ParseConfigWithOptions(connString string, options ParseConfigOptions) (*Con } } - batchInsertPendingSize := defaultPreparedBatchFlushSize + batchInsertPendingSize := defaultBatchInsertPendingSize if s, ok := config.RuntimeParams["batch_insert_pending_size"]; ok { delete(config.RuntimeParams, "batch_insert_pending_size") n, err := strconv.ParseInt(s, 10, 32) if err != nil { return nil, fbconn.NewParseConfigError(connString, "cannot parse batch_insert_pending_size", err) } + if n < 0 { + return nil, fbconn.NewParseConfigError(connString, "invalid batch_insert_pending_size", fmt.Errorf("must not be negative")) + } batchInsertPendingSize = int(n) } @@ -314,7 +317,8 @@ func ParseConfigWithOptions(connString string, options ParseConfigOptions) (*Con // Default: 512. // // - batch_insert_pending_size -// The maximum size of pending data in batch before [Conn.BatchInsert] will flush it to server. +// The client buffer threshold in bytes before [Conn.BatchInsert] flushes to the server. +// Zero flushes every row; negative values are invalid. This does not limit the server buffer. // Default: 128*1024. func ParseConfig(connString string) (*ConnConfig, error) { return ParseConfigWithOptions(connString, ParseConfigOptions{}) @@ -351,7 +355,6 @@ func connect(ctx context.Context, config *ConnConfig) (c *Conn, err error) { } c.preparedStatements = make(map[string]*fbconn.Statement) - c.preparedBatches = make(map[string]*PreparedBatch) c.doneChan = make(chan struct{}) c.closedChan = make(chan error) c.wbuf = make([]byte, 0, 1024) @@ -392,10 +395,6 @@ func (c *Conn) DropDatabase(ctx context.Context) error { // Named statements belong to the connection and can be reused in any query // execution mode until explicitly deallocated or the connection is closed. // -// If sql starts with "batch ", Prepare creates a named batch. -// Use [Conn.Exec] to add rows, [Conn.Flush] to send queued rows, and -// [Conn.ExecBatch] to execute the batch. -// // The underlying Firebird identifier for the prepared statement will be name if name != sql or a digest of sql if // name == sql. // @@ -406,10 +405,6 @@ func (c *Conn) Prepare(ctx context.Context, name, sql string) (sd *fbconn.Statem } func (c *Conn) prepareTx(ctx context.Context, tx *dbTx, name string, sql string) (*fbconn.StatementDescription, error) { - if normalizedSQL, ok := isBatchPrepareSQL(sql); ok { - return c.prepareNamedBatchTx(ctx, tx, name, sql, normalizedSQL) - } - st, err := c.internalPrepareForConnection(ctx, tx, name, sql) if err != nil { return nil, err @@ -436,67 +431,46 @@ func (c *Conn) PrepareBatchWithOptions(ctx context.Context, sql string, opts Bat } func (c *Conn) prepareBatchTx(ctx context.Context, tx *dbTx, sql string, opts BatchOptions) (*PreparedBatch, error) { - releaseInternalTx := c.shouldReleaseBatchInternalTx(tx) - st, err := c.internalPrepare(ctx, tx, "", sql) + cfg, err := normalizeBatchOptions(opts) if err != nil { - return nil, errors.Join(err, c.rollbackBatchInternalTx(ctx, releaseInternalTx)) - } - if err := validateBatchStatement(st.Description); err != nil { - var joinedErr = err - if deallocErr := st.Deallocate(ctx); deallocErr != nil { - joinedErr = errors.Join(joinedErr, deallocErr) - } - joinedErr = errors.Join(joinedErr, c.rollbackBatchInternalTx(ctx, releaseInternalTx)) - return nil, joinedErr + return nil, err } - - pb := &PreparedBatch{ - conn: c, - tx: tx, - sql: sql, - sd: st.Description, - autoFlushBytes: defaultPreparedBatchFlushSize, - batchCfg: normalizeBatchOptions(opts), + st, err := c.prepareBatchStatement(ctx, tx, sql) + if err == nil { + err = validateBatchStatement(st.Description) } - if tx != nil { - pb.st = st - return pb, nil + if err != nil && st != nil { + cleanupCtx, cancel := cleanupContext(ctx) + defer cancel() + if deallocErr := st.Deallocate(cleanupCtx); deallocErr != nil { + c.die() + err = operationerror.Join(err, deallocErr) + } } - - if err := errors.Join(st.Deallocate(ctx), c.rollbackBatchInternalTx(ctx, releaseInternalTx)); err != nil { + if err != nil { return nil, err } - return pb, nil -} - -// Flush sends queued rows for a named batch without executing it. -// -// Rows are queued for a named batch by calling [Conn.Exec] with the prepared -// batch name. Flush is a no-op when the named batch has no locally queued rows. -func (c *Conn) Flush(ctx context.Context, name string) error { - return c.flushTx(ctx, nil, name) + return &PreparedBatch{ + conn: c, + tx: tx, + st: st, + batchCfg: cfg, + }, nil } -// ExecBatch flushes queued rows for a named batch, executes it, and returns the result. -// -// Like [PreparedBatch.Exec], ExecBatch is atomic in autocommit mode: any -// per-row error rolls the whole batch back (no rows applied), in both -// stop-on-first and BatchOptions.ContinueOnError modes. -func (c *Conn) ExecBatch(ctx context.Context, name string) (BatchResult, error) { - return c.execBatchTx(ctx, nil, name) -} - -// CancelBatch discards every row queued for a named batch — both the local -// buffer and any rows already flushed to the server. The prepared batch -// statement is kept and can be reused via [Conn.Exec], [Conn.Flush], and -// [Conn.ExecBatch]. -// -// CancelBatch does not deallocate the named batch. Use [Conn.Deallocate] when -// the named batch is no longer needed. -func (c *Conn) CancelBatch(ctx context.Context, name string) error { - return c.cancelBatchTx(ctx, nil, name) +func (c *Conn) prepareBatchStatement(ctx context.Context, tx *dbTx, sql string) (st *fbconn.Statement, err error) { + if tx != nil { + return c.internalPrepare(ctx, tx, "", sql) + } + if c.prepareTracer != nil { + ctx = c.prepareTracer.TracePrepareStart(ctx, c, TracePrepareStartData{SQL: sql}) + defer func() { + c.prepareTracer.TracePrepareEnd(ctx, c, TracePrepareEndData{Err: err}) + }() + } + return c.fbConn.PrepareBatchStatement(ctx, sql) } func preparedStatementNameAndKey(name, sql string) (psName, psKey string) { @@ -507,38 +481,6 @@ func preparedStatementNameAndKey(name, sql string) (psName, psKey string) { return name, name } -func (c *Conn) prepareNamedBatchTx(ctx context.Context, tx *dbTx, name, originalSQL, normalizedSQL string) (*fbconn.StatementDescription, error) { - psName, psKey := preparedStatementNameAndKey(name, originalSQL) - - if existing := c.preparedBatches[psKey]; existing != nil { - if existing.sql != normalizedSQL { - return nil, fmt.Errorf("prepared query '%s' already exists", name) - } - if err := c.ensurePreparedBatchOwner(tx, psKey, existing); err != nil { - return nil, err - } - return existing.sd, nil - } - if _, exists := c.preparedStatements[psKey]; exists { - return nil, fmt.Errorf("prepared query '%s' already exists", name) - } - - pb, err := c.prepareBatchTx(ctx, tx, normalizedSQL, BatchOptions{}) - if err != nil { - return nil, err - } - pb.name = psName - pb.autoFlushBytes = defaultPreparedBatchFlushSize - pb.sd.Name = psName - if pb.st != nil { - pb.st.Description.Name = psName - } - if psKey != "" { - c.preparedBatches[psKey] = pb - } - return pb.sd, nil -} - type statementScope uint8 const ( @@ -567,9 +509,6 @@ func (c *Conn) internalPrepareWithScope( var st *fbconn.Statement if name != "" { - if _, ok := c.preparedBatches[name]; ok { - return nil, fmt.Errorf("prepared query '%s' already exists", name) - } var ok bool if st, ok = c.preparedStatements[name]; ok { if sql != c.preparedStatements[name].Description.SQL { @@ -610,22 +549,12 @@ func (c *Conn) internalPrepareWithScope( return st, nil } -// Deallocate releases a prepared statement, an automatically cached statement, -// or a named batch. Automatically cached statements can be identified by their -// SQL text. +// Deallocate releases a prepared statement or an automatically cached statement. +// Automatically cached statements can be identified by their SQL text. // If name identifies both a prepared statement and cached SQL, Deallocate // releases both statements. // Calling Deallocate on a non-existent object will succeed. func (c *Conn) Deallocate(ctx context.Context, name string) error { - if pb := c.preparedBatches[name]; pb != nil { - err := pb.Close(ctx) - if err != nil { - return err - } - delete(c.preparedBatches, name) - return nil - } - preparedStatement := c.preparedStatements[name] var cachedStatement *fbconn.Statement if c.statementCache != nil { @@ -690,7 +619,7 @@ func (c *Conn) deallocatePreparedStatement(ctx context.Context, st *fbconn.State return nil } -// DeallocateAll releases all prepared statements and named batches. +// DeallocateAll releases all prepared statements. // It resets the statement cache after all deallocations succeed. On error, // statements that have not been released remain registered for a later retry. func (c *Conn) DeallocateAll(ctx context.Context) error { @@ -699,13 +628,7 @@ func (c *Conn) DeallocateAll(ctx context.Context) error { return err } } - for name := range c.preparedBatches { - if err := c.Deallocate(ctx, name); err != nil { - return err - } - } c.preparedStatements = map[string]*fbconn.Statement{} - c.preparedBatches = map[string]*PreparedBatch{} if c.config.StatementCacheCapacity > 0 { c.statementCache = stmtcache.NewLRUCache(c.config.StatementCacheCapacity) } @@ -769,6 +692,9 @@ func (c *Conn) Config() *ConnConfig { // Exec executes sql inside internal transaction. sql can be either a prepared statement name or an SQL string. arguments should be referenced // positionally from the sql string as ?. +// Pass one [BatchRows] argument to execute a complete batch with default options. +// No rows remain queued after Exec returns. In autocommit mode, per-row errors +// roll back the batch; an explicit transaction remains under caller control. func (c *Conn) Exec(ctx context.Context, sql string, arguments ...any) (fbconn.QueryStatus, error) { return c.execTx(ctx, nil, sql, arguments...) } @@ -823,11 +749,10 @@ optionLoop: invalidateSQL = sql } - if pb := c.preparedBatches[sql]; pb != nil { - if err := c.ensurePreparedBatchOwner(tx, sql, pb); err != nil { - return fbconn.QueryStatus{}, err + if len(arguments) == 1 { + if rows, ok := arguments[0].(BatchRows); ok { + return c.execBatchRows(ctx, tx, sql, mode, rows) } - return c.execPreparedBatch(ctx, pb, arguments) } defer func() { @@ -948,84 +873,6 @@ func (c *Conn) execPrepared(ctx context.Context, tx *dbTx, st *fbconn.Statement, return result.QueryStatus, result.Err } -func (c *Conn) execPreparedBatch(ctx context.Context, pb *PreparedBatch, arguments []any) (fbconn.QueryStatus, error) { - if err := pb.addEncoded(ctx, arguments, true); err != nil { - return fbconn.QueryStatus{}, err - } - return fbconn.QueryStatus{}, nil -} - -func (c *Conn) flushTx(ctx context.Context, tx *dbTx, name string) error { - pb := c.preparedBatches[name] - if pb == nil { - return fmt.Errorf("prepared batch %q not found", name) - } - if err := c.ensurePreparedBatchOwner(tx, name, pb); err != nil { - return err - } - return pb.Flush(ctx) -} - -func (c *Conn) execBatchTx(ctx context.Context, tx *dbTx, name string) (BatchResult, error) { - pb := c.preparedBatches[name] - if pb == nil { - return BatchResult{}, fmt.Errorf("prepared batch %q not found", name) - } - if err := c.ensurePreparedBatchOwner(tx, name, pb); err != nil { - return BatchResult{}, err - } - return pb.Exec(ctx) -} - -func (c *Conn) cancelBatchTx(ctx context.Context, tx *dbTx, name string) error { - pb := c.preparedBatches[name] - if pb == nil { - return fmt.Errorf("prepared batch %q not found", name) - } - if err := c.ensurePreparedBatchOwner(tx, name, pb); err != nil { - return err - } - return pb.Cancel(ctx) -} - -func (c *Conn) ensurePreparedBatchOwner(tx *dbTx, name string, pb *PreparedBatch) error { - if pb == nil { - return fmt.Errorf("prepared batch %q not found", name) - } - if pb.tx == nil { - if tx != nil { - return fmt.Errorf("prepared batch %q belongs to connection scope", name) - } - return nil - } - if pb.tx.closed { - return ErrTxClosed - } - if tx == nil || tx != pb.tx { - return fmt.Errorf("prepared batch %q belongs to a different transaction", name) - } - return nil -} - -func (c *Conn) closePreparedBatchesForTx(ctx context.Context, tx *dbTx) error { - var joinedErr error - for name, pb := range c.preparedBatches { - if pb == nil || pb.tx != tx { - continue - } - if err := pb.Close(ctx); err != nil { - joinedErr = errors.Join(joinedErr, err) - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - continue - } - delete(c.preparedBatches, name) - continue - } - delete(c.preparedBatches, name) - } - return joinedErr -} - func (c *Conn) getRows(ctx context.Context, sql string, args []any) *baseRows { r := &baseRows{} @@ -1138,10 +985,11 @@ optionLoop: c.eqb.reset() rows := c.getRows(ctx, sql, args) - if pb := c.preparedBatches[sql]; pb != nil { - err := fmt.Errorf("prepared batch %q cannot be queried; use Exec/Flush/ExecBatch", sql) - rows.fatal(err) - return rows, err + if len(args) == 1 { + if _, ok := args[0].(BatchRows); ok { + rows.fatal(fmt.Errorf("BatchRows must be executed with Exec")) + return rows, rows.Err() + } } st, temporary, err := c.resolveStatement(ctx, tx, sql, mode) diff --git a/conn_doc.go b/conn_doc.go index 5de2bc2..1a24fe7 100644 --- a/conn_doc.go +++ b/conn_doc.go @@ -112,8 +112,8 @@ are gone, while the failed and unprocessed entries retain their registry/cache ownership so cleanup can be retried. If the connection remains open, an error must not leave a live handle detached from its old LRU. -Named batches use preparedBatches and have their own execution and transaction -ownership rules. In particular, a batch prepared through Tx belongs to that -transaction and is closed when it ends; the ordinary statement rules above do -not change batch ownership. +Batch objects created by Conn.PrepareBatch or Tx.PrepareBatch are owned by the +caller and are not registered in preparedStatements or the LRU. Always call +PreparedBatch.Close to release their batch and statement handles. A batch +created through Tx.PrepareBatch cannot execute after that transaction ends. */ diff --git a/conn_internal_test.go b/conn_internal_test.go index f7c19c2..bada29c 100644 --- a/conn_internal_test.go +++ b/conn_internal_test.go @@ -35,7 +35,6 @@ func TestCacheStatementModeRequiresCache(t *testing.T) { DefaultQueryExecMode: QueryExecModeCacheStatement, }, preparedStatements: make(map[string]*fbconn.Statement), - preparedBatches: make(map[string]*PreparedBatch), } _, err := conn.Exec(context.Background(), "select 1 from rdb$database") diff --git a/conn_test.go b/conn_test.go index cbfbfaa..60cf4ed 100644 --- a/conn_test.go +++ b/conn_test.go @@ -310,6 +310,23 @@ func TestParseConfigExtractsDefaultQueryExecMode(t *testing.T) { } } +func TestParseConfigBatchInsertPendingSize(t *testing.T) { + t.Parallel() + for _, tt := range []struct { + connString string + want int + }{ + {"", 128 * 1024}, + {"batch_insert_pending_size=0", 0}, + {"batch_insert_pending_size=42", 42}, + } { + config, err := fbx.ParseConfig(tt.connString) + require.NoError(t, err) + require.Equal(t, tt.want, config.BatchInsertPendingSize) + require.NotContains(t, config.RuntimeParams, "batch_insert_pending_size") + } +} + func TestParseConfigErrors(t *testing.T) { t.Parallel() @@ -318,6 +335,8 @@ func TestParseConfigErrors(t *testing.T) { expectedErrSubstring string }{ {"default_query_exec_mode=does_not_exist", "does_not_exist"}, + {"batch_insert_pending_size=-1", "must not be negative"}, + {"batch_insert_pending_size=2147483648", "cannot parse batch_insert_pending_size"}, } { config, err := fbx.ParseConfig(tt.connString) require.Nil(t, config) @@ -671,53 +690,6 @@ func TestPrepareBatchUpdateAndDelete(t *testing.T) { }) } -func TestPrepareNamedBatchUpdateAndDelete(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - fbxtest.RunWithQueryExecModes(ctx, t, defaultConnTestRunner, nil, func(ctx context.Context, t testing.TB, conn *fbx.Conn) { - skipIfBatchUnsupported(ctx, t, conn) - table := batchTestTableName(t, "fbx_nbv") - updateName := batchTestTableName(t, "upd_batch") - deleteName := batchTestTableName(t, "del_batch") - - _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, note varchar(50))") - require.NoError(t, err) - seedBatchDMLRows(t, ctx, conn, table) - - _, err = conn.Prepare(ctx, updateName, "batch update "+table+" set note = ? where id = ?") - require.NoError(t, err) - defer func() { require.NoError(t, conn.Deallocate(ctx, updateName)) }() - - _, err = conn.Exec(ctx, updateName, "one-named", 1) - require.NoError(t, err) - _, err = conn.Exec(ctx, updateName, "two-named", 2) - require.NoError(t, err) - - updateResult, err := conn.ExecBatch(ctx, updateName) - require.NoError(t, err) - require.EqualValues(t, 2, updateResult.Affected) - - _, err = conn.Prepare(ctx, deleteName, "batch delete from "+table+" where id = ?") - require.NoError(t, err) - defer func() { require.NoError(t, conn.Deallocate(ctx, deleteName)) }() - - _, err = conn.Exec(ctx, deleteName, 3) - require.NoError(t, err) - _, err = conn.Exec(ctx, deleteName, 4) - require.NoError(t, err) - - deleteResult, err := conn.ExecBatch(ctx, deleteName) - require.NoError(t, err) - require.EqualValues(t, 2, deleteResult.Affected) - - require.Equal(t, []batchDMLRow{ - {ID: 1, Note: "one-named"}, - {ID: 2, Note: "two-named"}, - }, loadBatchDMLRows(t, ctx, conn, table)) - }) -} - func TestPrepareBatchRejectsResultSetStatements(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() @@ -725,7 +697,6 @@ func TestPrepareBatchRejectsResultSetStatements(t *testing.T) { fbxtest.RunWithQueryExecModes(ctx, t, defaultConnTestRunner, nil, func(ctx context.Context, t testing.TB, conn *fbx.Conn) { skipIfBatchUnsupported(ctx, t, conn) table := batchTestTableName(t, "fbx_brr") - badBatchName := batchTestTableName(t, "bad_batch") _, err := conn.PrepareBatch(ctx, "select cast(? as integer) from rdb$database") require.ErrorContains(t, err, "batch supports only insert, update, and delete statements without result sets") @@ -739,12 +710,6 @@ func TestPrepareBatchRejectsResultSetStatements(t *testing.T) { _, err = conn.PrepareBatch(ctx, "insert into "+table+" (id, note) values (1, 'no params')") require.ErrorContains(t, err, "statement used in batch must have parameters") - _, err = conn.Prepare(ctx, badBatchName, "batch select cast(? as integer) from rdb$database") - require.ErrorContains(t, err, "batch supports only insert, update, and delete statements without result sets") - - _, err = conn.Prepare(ctx, badBatchName+"_noparams", "batch update "+table+" set note = 'no params' where id = 1") - require.ErrorContains(t, err, "statement used in batch must have parameters") - ensureConnValid(t, conn) }) } @@ -756,7 +721,6 @@ func TestPrepareBatchRejectsBlobParameters(t *testing.T) { fbxtest.RunWithQueryExecModes(ctx, t, defaultConnTestRunner, nil, func(ctx context.Context, t testing.TB, conn *fbx.Conn) { skipIfBatchUnsupported(ctx, t, conn) table := batchTestTableName(t, "fbx_bblob") - badBatchName := batchTestTableName(t, "bad_blob_batch") _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, payload blob sub_type 0)") require.NoError(t, err) @@ -764,9 +728,6 @@ func TestPrepareBatchRejectsBlobParameters(t *testing.T) { _, err = conn.PrepareBatch(ctx, "insert into "+table+" (id, payload) values (?, ?)") require.ErrorContains(t, err, "batch does not support blob parameters") - _, err = conn.Prepare(ctx, badBatchName, "batch insert into "+table+" (id, payload) values (?, ?)") - require.ErrorContains(t, err, "batch does not support blob parameters") - ensureConnValid(t, conn) }) } @@ -778,7 +739,6 @@ func TestPrepareBatchRejectsArrayParameters(t *testing.T) { fbxtest.RunWithQueryExecModes(ctx, t, defaultConnTestRunner, nil, func(ctx context.Context, t testing.TB, conn *fbx.Conn) { skipIfBatchUnsupported(ctx, t, conn) table := batchTestTableName(t, "fbx_barr") - badBatchName := batchTestTableName(t, "bad_array_batch") _, err := conn.Exec(ctx, "recreate table "+table+" (id integer not null primary key, val smallint[1:3])") require.NoError(t, err) @@ -786,9 +746,6 @@ func TestPrepareBatchRejectsArrayParameters(t *testing.T) { _, err = conn.PrepareBatch(ctx, "insert into "+table+" (id, val) values (?, ?)") require.ErrorContains(t, err, "batch does not support array parameters") - _, err = conn.Prepare(ctx, badBatchName, "batch insert into "+table+" (id, val) values (?, ?)") - require.ErrorContains(t, err, "batch does not support array parameters") - ensureConnValid(t, conn) }) } diff --git a/doc.go b/doc.go index e4dd234..e914319 100644 --- a/doc.go +++ b/doc.go @@ -16,17 +16,32 @@ created with [ParseConfig] and then passed to [ConnectConfig]. fbx exposes Firebird native batch DML for INSERT, UPDATE, and DELETE statements with parameters and without result sets. Blob and array parameters are rejected -by the batch API. There are two batch styles: object batches and named batches. +by the batch API. -# Object Batches +Pass one [BatchRows] argument to [Conn.Exec] or [Tx.Exec] to execute a complete +batch with default options. This also works through database/sql ExecContext, +including prepared statements. -Object batches are created with [Conn.PrepareBatch] or -[Conn.PrepareBatchWithOptions]. Rows are added with [PreparedBatch.Add]. Sending -is manual: [PreparedBatch.Flush] sends queued rows to the server-side batch + _, err := conn.Exec(ctx, "insert into t(id, note) values (?, ?)", + BatchRows{{1, "one"}, {2, "two"}}) + +Each call sends and executes the whole set, then releases its server batch. +No rows remain queued for another call. Encoded rows must fit in the server +batch buffer; the driver does not split executions into chunks. For larger +loads, submit portions in an explicit transaction and commit or roll back +the transaction yourself. Batches use the same transaction rules described below. + +Use [Conn.PrepareBatch] or [Tx.PrepareBatch] to create a batch object, or their +PrepareBatchWithOptions variants to configure it. Rows are added with +[PreparedBatch.Add]. [PreparedBatch.Flush] sends rows to the server-side batch buffer without executing them, and [PreparedBatch.Exec] flushes any remaining rows and executes the batch. [PreparedBatch.PendingBytes] can be used to decide when to flush queued rows. +[PreparedBatch.Info] queries the server's buffer capacity and current data sizes +(protocol 17 or later). It does not flush or execute rows. These sizes describe +server storage and can differ from the local encoded size in PendingBytes. + pb, err := conn.PrepareBatch(context.Background(), "insert into t(id, note) values (?, ?)") if err != nil { return err @@ -55,36 +70,10 @@ Always call [PreparedBatch.Close] when the batch is no longer needed. locally queued rows and rows already flushed to the server while keeping the batch reusable. -# Named Batches - -Named batches use [Conn.Prepare] with SQL that starts with "batch ". Rows are -added with [Conn.Exec] using the prepared batch name. Adding a row does not -execute the batch; it only encodes the row and may auto-flush queued rows to the -server. Use [Conn.Flush] to send queued rows explicitly and [Conn.ExecBatch] to -execute the batch. Named batches are released with [Conn.Deallocate]. - - _, err := conn.Prepare(context.Background(), "b1", "batch insert into t(id, note) values (?, ?)") - if err != nil { - return err - } - defer conn.Deallocate(context.Background(), "b1") - - for _, row := range rows { - if _, err := conn.Exec(context.Background(), "b1", row.ID, row.Note); err != nil { - return err - } - } - - res, err := conn.ExecBatch(context.Background(), "b1") - if err != nil { - return err - } - _ = res - # Batch Results and Errors -[PreparedBatch.Exec] and [Conn.ExecBatch] return a [BatchResult]. If the server -reports per-row errors, the error can be inspected as a [BatchExecutionError]. +[PreparedBatch.Exec] returns a [BatchResult]. If the server reports per-row +errors, the error can be inspected as a [BatchExecutionError]. BatchOptions.ContinueOnError asks the server to continue after row errors and report more of them; it does not change transaction atomicity. diff --git a/examples_test.go b/examples_test.go index bf8fee7..1ebb24a 100644 --- a/examples_test.go +++ b/examples_test.go @@ -194,67 +194,6 @@ func ExampleConn_PrepareBatch() { // rows: 2 } -func ExampleConn_ExecBatch() { - ctx := context.Background() - dsn := "firebird://sysdba:masterkey@localhost:3050/employee" - - conn, err := fbx.Connect(ctx, dsn) - if err != nil { - fmt.Println(err) - return - } - defer func() { - _ = conn.Close(ctx) - }() - - if !exampleBatchSupported(ctx, conn) { - fmt.Println("rows: 2") - return - } - - _, err = conn.Exec(ctx, "recreate table fbx_example_named_batch(id integer not null primary key, note varchar(20))") - if err != nil { - fmt.Println(err) - return - } - - _, err = conn.Prepare(ctx, "audit_batch", "batch insert into fbx_example_named_batch(id, note) values (?, ?)") - if err != nil { - fmt.Println(err) - return - } - defer func() { - _ = conn.Deallocate(ctx, "audit_batch") - }() - - _, err = conn.Exec(ctx, "audit_batch", int64(1), "one") - if err != nil { - fmt.Println(err) - return - } - _, err = conn.Exec(ctx, "audit_batch", int64(2), "two") - if err != nil { - fmt.Println(err) - return - } - - _, err = conn.ExecBatch(ctx, "audit_batch") - if err != nil { - fmt.Println(err) - return - } - - var count int - if err := conn.QueryRow(ctx, "select count(*) from fbx_example_named_batch").Scan(&count); err != nil { - fmt.Println(err) - return - } - fmt.Printf("rows: %d\n", count) - - // Output: - // rows: 2 -} - func ExampleNamedArgs() { ctx := context.Background() dsn := "firebird://sysdba:masterkey@localhost:3050/employee" diff --git a/fbconn/batch.go b/fbconn/batch.go index b9eb4bf..b209389 100644 --- a/fbconn/batch.go +++ b/fbconn/batch.go @@ -4,24 +4,40 @@ import ( "context" "errors" "fmt" + "math" "github.com/firebirdsql/fbx/fbproto" "github.com/firebirdsql/fbx/internal/operationerror" ) +// BatchConfig configures a server batch created by [Statement.NewBatch]. type BatchConfig struct { - MultiError bool + // MultiError continues execution after per-message errors. + MultiError bool + // RecordCounts requests affected row counts for each message. RecordCounts bool + // DetailedErrs requests a detailed error limit: -1 uses the server default, + // 0 disables details. Values up to math.MaxInt32 are accepted; the server may cap them. DetailedErrs int - BufferBytes int - BlobPolicy byte + // BufferBytes requests a server buffer size: -1 uses the server default, + // 0 requests the server maximum. Positive values up to math.MaxInt32 may be capped by the server. + BufferBytes int + // BlobPolicy is one of fbproto.BatchBlobNone, BatchBlobIDEngine, + // BatchBlobIDUser, or BatchBlobStream. Zero preserves BatchBlobNone. + BlobPolicy byte } -func (cfg BatchConfig) normalize() BatchConfig { - if cfg.BlobPolicy == 0 { - cfg.BlobPolicy = fbproto.BatchBlobIDUser +func (cfg BatchConfig) validate() error { + if cfg.BufferBytes < -1 || cfg.BufferBytes > math.MaxInt32 { + return fmt.Errorf("batch buffer size must be between -1 and %d (got %d)", math.MaxInt32, cfg.BufferBytes) } - return cfg + if cfg.DetailedErrs < -1 || cfg.DetailedErrs > math.MaxInt32 { + return fmt.Errorf("batch detailed error limit must be between -1 and %d (got %d)", math.MaxInt32, cfg.DetailedErrs) + } + if cfg.BlobPolicy > fbproto.BatchBlobStream { + return fmt.Errorf("invalid batch blob policy %d", cfg.BlobPolicy) + } + return nil } func (cfg BatchConfig) writeParameterBuffer(pb *fbproto.BatchPBWriter) { @@ -51,9 +67,20 @@ type BatchError struct { Err error } +// BatchInfo describes the current server buffers and BLOB format of a batch. +// Byte counts describe server storage, not the encoded wire data or client memory. +type BatchInfo struct { + BufferBytes int // Actual capacity of each server buffer (parameter data and BLOB data). + DataBytes int // Parameter data currently buffered on the server, including alignment. + BlobBytes int // BLOB data currently buffered on the server. + BlobAlignment int // Required alignment of BLOB entries in a batch stream, in bytes. + BlobHeaderBytes int // Size of a BLOB entry header in a batch stream, in bytes. +} + var errBatchExecutionFailed = errors.New("batch execution failed") type BatchCompletion struct { + // TotalRecords is the number of processed batch messages, not affected table rows. TotalRecords uint32 UpdateCounts []int64 Errors []BatchError @@ -63,17 +90,22 @@ func (c *BatchCompletion) HasErrors() bool { return c != nil && len(c.Errors) > 0 } +// Affected returns the sum of known update counts, excluding failed messages. +// It returns -1 if counts are missing or unknown, and 0 for an empty or nil completion. +// The result describes this execution, regardless of a later commit or rollback. func (c *BatchCompletion) Affected() int64 { if c == nil { return 0 } - if len(c.UpdateCounts) == 0 { - return int64(c.TotalRecords) + if uint64(len(c.UpdateCounts)) != uint64(c.TotalRecords) { + return -1 } var affected int64 for _, n := range c.UpdateCounts { - if n > 0 { + if n >= 0 { affected += n + } else if n != fbproto.BatchExecuteFailed { + return -1 } } return affected @@ -87,11 +119,51 @@ const ( batchStateClosed ) +type batchBufferState int + +const ( + batchBufferEmpty batchBufferState = iota + batchBufferPending + batchBufferNeedsReset +) + type Batch struct { - st *Statement - state batchState - cfg BatchConfig - err error + st *Statement + state batchState + cfg BatchConfig + err error + bufferState batchBufferState // Server queue, independent of the batch's lifetime. +} + +// HasPending reports whether the server queue contains rows or its contents +// are uncertain after an unsuccessful exchange. +func (b *Batch) HasPending() bool { + return b != nil && b.bufferState != batchBufferEmpty +} + +// NeedsReset reports an attempted send or execution without a confirmed outcome. +// Call Cancel before sending or executing more rows, or Close to release the batch. +func (b *Batch) NeedsReset() bool { + return b != nil && b.bufferState == batchBufferNeedsReset +} + +// PrepareBatchStatement prepares a connection-scoped statement for batching. +// In autocommit mode, a transaction started only for this preparation is rolled +// back before returning. An existing transaction is left to its owner. +func (fbConn *FbConn) PrepareBatchStatement(ctx context.Context, sql string) (*Statement, error) { + startedTx := fbConn.internalTransaction == nil || fbConn.internalTransaction.Closed + tx, err := fbConn.ensureInternalTransaction(ctx, nil, false) + if err != nil { + return nil, err + } + st, err := fbConn.prepareTx(ctx, tx, sql, statementScopeConnection) + if startedTx { + err = operationerror.Join(err, fbConn.rollbackOwnedInternalTx(ctx, tx)) + } + if err != nil { + return nil, err + } + return st, nil } func (b *Batch) isClosed() bool { @@ -108,6 +180,9 @@ func (b *Batch) checkCreate() error { if b.state == batchStateClosed { return fmt.Errorf("batch is closed") } + if b.NeedsReset() { + return fmt.Errorf("batch requires Cancel after an unsuccessful exchange") + } if b.state == batchStateCreated { return nil } @@ -127,28 +202,28 @@ func (b *Batch) createOnWire(ctx context.Context, parameterBuffer []byte) error return b.withConn(ctx, func() error { batchBLR, err := b.st.Description.StatementInfo.GetParametersBlr(nil) if err != nil { - b.st.conn.asyncClose() return err } - if err := b.st.conn.client.WriteMessage(&fbproto.OpBatchCreate{ - StatementHandle: b.st.Description.StatementHandle, - BatchBLR: batchBLR, - MessageLength: fbproto.CalculateBatchMessageLength(b.st.Description.StatementInfo.ParametersDescriptors), - ParameterBuffer: parameterBuffer, + if err := b.st.conn.withWriteLockAndFlush(func() error { + if err := b.st.conn.client.WriteMessage(&fbproto.OpBatchCreate{ + StatementHandle: b.st.Description.StatementHandle, + BatchBLR: batchBLR, + MessageLength: fbproto.CalculateBatchMessageLength(b.st.Description.StatementInfo.ParametersDescriptors), + ParameterBuffer: parameterBuffer, + }); err != nil { + return err + } + return b.st.conn.client.WriteMessage(&fbproto.OpPing{}) }); err != nil { b.st.conn.asyncClose() return err } - if err := b.st.conn.client.WriteMessage(&fbproto.OpPing{}); err != nil { - b.st.conn.asyncClose() - return err - } - if err := b.st.conn.flushWithPotentialWriteReadDeadlock(); err != nil { - b.st.conn.asyncClose() - return err + created, err := b.receiveTwoGenericResponses(ctx) + if created { + b.state = batchStateCreated } - return b.receiveTwoGenericResponses(ctx) + return err }) } @@ -160,12 +235,88 @@ func (b *Batch) Create(ctx context.Context) error { return nil } - if err := b.createOnWire(ctx, b.cfg.parameterBuffer()); err != nil { + return b.createOnWire(ctx, b.cfg.parameterBuffer()) +} + +// Info reads server batch information, creating the batch if necessary. +// It requires protocol version 17 or later and does not send or execute rows. +func (b *Batch) Info(ctx context.Context) (BatchInfo, error) { + if err := b.checkCreate(); err != nil { + return BatchInfo{}, err + } + if b.st.conn.protocolVersion < fbproto.PROTOCOL_VERSION17 { + return BatchInfo{}, fmt.Errorf("batch info is not supported by protocol version %d", b.st.conn.protocolVersion) + } + if err := b.Create(ctx); err != nil { + return BatchInfo{}, err + } + var info BatchInfo + err := b.withConn(ctx, func() error { + msgBuilder := func() fbproto.ClientMessage { + return &fbproto.OpInfoBatch{ + StatementHandle: b.st.Description.StatementHandle, + Items: []byte{fbproto.BatchInfoBufferBytes, fbproto.BatchInfoDataBytes, + fbproto.BatchInfoBlobsBytes, fbproto.BatchInfoBlobAlign, fbproto.BatchInfoBlobHeader}, + BufferLength: 64, // Five integer items and the end marker fit without continuation. + } + } + var data []byte + // Capture the complete response before parsing; a truncated payload is an error, + // not a reason to retry this fixed-size information request. + if err := b.st.conn.getServerInfo(msgBuilder, nil, func(buf []byte) error { + data = buf + return nil + }); err != nil { + return err + } + var err error + info, err = parseBatchInfo(data) return err + }) + if err != nil { + return BatchInfo{}, err } + return info, nil +} - b.state = batchStateCreated - return nil +func parseBatchInfo(data []byte) (BatchInfo, error) { + var info BatchInfo + fields := [...]*int{&info.BufferBytes, &info.DataBytes, &info.BlobBytes, &info.BlobAlignment, &info.BlobHeaderBytes} + var seen [len(fields)]bool + r := NewServerInfoBufferReader(data) + for { + item, err := r.ReadByte() + if err != nil { + return BatchInfo{}, fmt.Errorf("incomplete batch info: %w", err) + } + switch item { + case fbproto.Isc_info_end: + for i, present := range seen { + // Firebird omits the BLOB byte count when the BLOB buffer is empty. + if !present && i != fbproto.BatchInfoBlobsBytes-fbproto.BatchInfoBufferBytes { + return BatchInfo{}, fmt.Errorf("missing batch info item %d", i+fbproto.BatchInfoBufferBytes) + } + } + return info, nil + case fbproto.Isc_info_truncated: + return BatchInfo{}, fmt.Errorf("batch info is truncated") + case fbproto.Isc_info_error: + return BatchInfo{}, fmt.Errorf("server could not provide batch info") + } + if item < fbproto.BatchInfoBufferBytes || item > fbproto.BatchInfoBlobHeader { + return BatchInfo{}, fmt.Errorf("unexpected batch info item %d", item) + } + value, err := r.ReadInt64Value() + if err != nil { + return BatchInfo{}, fmt.Errorf("batch info item %d: %w", item, err) + } + if value < 0 || value > math.MaxInt32 { + return BatchInfo{}, fmt.Errorf("invalid batch info value %d for item %d", value, item) + } + index := item - fbproto.BatchInfoBufferBytes + *fields[index] = int(value) + seen[index] = true + } } func (b *Batch) Send(ctx context.Context, rows [][]byte) error { @@ -177,23 +328,24 @@ func (b *Batch) Send(ctx context.Context, rows [][]byte) error { } return b.withConn(ctx, func() error { - if err := b.st.conn.client.WriteMessage(&fbproto.OpBatchMsg{ - StatementHandle: b.st.Description.StatementHandle, - Rows: rows, + b.bufferState = batchBufferNeedsReset + if err := b.st.conn.withWriteLockAndFlush(func() error { + if err := b.st.conn.client.WriteMessage(&fbproto.OpBatchMsg{ + StatementHandle: b.st.Description.StatementHandle, + Rows: rows, + }); err != nil { + return err + } + return b.writeBatchSyncMessage() }); err != nil { b.st.conn.asyncClose() return err } - if err := b.writeBatchSyncMessage(); err != nil { - b.st.conn.asyncClose() - return err + _, err := b.receiveTwoGenericResponses(ctx) + if err == nil { + b.bufferState = batchBufferPending } - - if err := b.st.conn.flushWithPotentialWriteReadDeadlock(); err != nil { - b.st.conn.asyncClose() - return err - } - return b.receiveTwoGenericResponses(ctx) + return err }) } @@ -215,37 +367,43 @@ func (b *Batch) Execute(ctx context.Context, tx *FBTransaction) (*BatchCompletio } var completion *BatchCompletion - executeAttempted := false err = b.withConn(ctx, func() error { - executeAttempted = true - if err := b.st.conn.client.WriteMessage(&fbproto.OpBatchExec{ - StatementHandle: b.st.Description.StatementHandle, - TransactionHandle: execTx.Handle, + b.bufferState = batchBufferNeedsReset + if err := b.st.conn.withWriteLockAndFlush(func() error { + return b.st.conn.client.WriteMessage(&fbproto.OpBatchExec{ + StatementHandle: b.st.Description.StatementHandle, + TransactionHandle: execTx.Handle, + }) }); err != nil { b.st.conn.asyncClose() return err } - if err := b.st.conn.flushWithPotentialWriteReadDeadlock(); err != nil { - b.st.conn.asyncClose() - return err - } completion, err = b.receiveCompletion(ctx) + if completion != nil { + // A completion consumes the entire queue, including skipped rows. + // A later rollback or commit failure does not restore that queue. + b.bufferState = batchBufferEmpty + } return err }) - if err != nil { - if executeAttempted { - return nil, operationerror.WithRetry(err, false) + if completion.HasErrors() && ctx.Err() != nil { + err = operationerror.Join(errBatchExecutionFailed, normalizeTimeoutError(ctx, ctx.Err())) + } + if err != nil || completion.HasErrors() { + if rbErr := b.st.conn.rollbackOwnedInternalTx(ctx, execTx); rbErr != nil { + if err == nil { + err = errBatchExecutionFailed + } + err = operationerror.Join(err, fmt.Errorf("rollback internal tx: %w", rbErr)) } - return nil, err + return completion, err } if execTx == b.st.conn.internalTransaction && b.st.conn.config.Autocommit && !b.st.conn.explicitTransaction { - if completion.HasErrors() { - if err := b.st.conn.rollbackInternalTxIfAutocommitOnError(ctx, execTx, errBatchExecutionFailed); err != nil { - return completion, operationerror.Join(errBatchExecutionFailed, fmt.Errorf("rollback internal tx: %w", err)) - } - } else if err := b.st.conn.commitInternalTx(context.Background(), execTx); err != nil { + commitCtx, cancel := cleanupContext(ctx) + defer cancel() + if err := b.st.conn.commitInternalTx(commitCtx, execTx); err != nil { return completion, err } } @@ -269,8 +427,9 @@ func (b *Batch) Cancel(ctx context.Context) error { } // Close sends [fbproto.Op_batch_rls] to the server, releasing the server-side -// batch context entirely. The [Batch] must not be used after [Batch.Close] -// returns. +// batch context entirely. A confirmed release closes the batch even if the +// following sync fails. An unconfirmed Close can be retried while the connection +// remains usable. func (b *Batch) Close(ctx context.Context) error { if b == nil || b.state == batchStateClosed { return nil @@ -289,29 +448,36 @@ func (b *Batch) Close(ctx context.Context) error { func (b *Batch) sendReleaseOp(ctx context.Context, op int) error { return b.withConn(ctx, func() error { - if err := b.st.conn.client.WriteMessage(&fbproto.OpBatchRelease{ - Kind: op, - StatementHandle: b.st.Description.StatementHandle, - }); err != nil { - b.st.conn.asyncClose() - return err + if op == fbproto.Op_batch_cancel { + b.bufferState = batchBufferNeedsReset } - if err := b.st.conn.client.WriteMessage(&fbproto.OpPing{}); err != nil { + if err := b.st.conn.withWriteLockAndFlush(func() error { + if err := b.st.conn.client.WriteMessage(&fbproto.OpBatchRelease{ + Kind: op, + StatementHandle: b.st.Description.StatementHandle, + }); err != nil { + return err + } + return b.st.conn.client.WriteMessage(&fbproto.OpPing{}) + }); err != nil { b.st.conn.asyncClose() return err } - if err := b.st.conn.flushWithPotentialWriteReadDeadlock(); err != nil { - b.st.conn.asyncClose() - return err + released, err := b.receiveTwoGenericResponses(ctx) + if released { + b.bufferState = batchBufferEmpty + if op == fbproto.Op_batch_rls { + b.markClosed() + } } - return b.receiveTwoGenericResponses(ctx) + return err }) } func (b *Batch) receiveGenericResponse(ctx context.Context) error { msg, err := b.st.conn.receiveMessage() if err != nil { - b.st.conn.asyncClose() + b.st.conn.asyncCloseAfterRead(err) return normalizeTimeoutError(ctx, err) } switch m := msg.(type) { @@ -320,21 +486,35 @@ func (b *Batch) receiveGenericResponse(ctx context.Context) error { case *fbproto.ErrorResponse: return ErrorResponseToFbError(m) default: + b.st.conn.asyncClose() return fmt.Errorf("received unexpected batch sync response %T", msg) } } -func (b *Batch) receiveTwoGenericResponses(ctx context.Context) error { +func (b *Batch) receiveTwoGenericResponses(ctx context.Context) (bool, error) { firstErr := b.receiveGenericResponse(ctx) + + if b.st.conn.IsClosed() { + return firstErr == nil, firstErr + } secondErr := b.receiveGenericResponse(ctx) - return errors.Join(firstErr, secondErr) + return firstErr == nil, errors.Join(firstErr, secondErr) } -func (b *Batch) withConn(ctx context.Context, fn func() error) error { +func (b *Batch) withConn(ctx context.Context, fn func() error) (err error) { if err := b.st.conn.lock(); err != nil { return err } defer b.st.conn.unlock() + defer func() { + if err != nil && ctx.Err() != nil && !errors.Is(err, ctx.Err()) { + err = operationerror.Join(err, normalizeTimeoutError(ctx, ctx.Err())) + } + }() + + if err := b.st.conn.beforeRequest(); err != nil { + return err + } if ctx != context.Background() { select { @@ -346,32 +526,36 @@ func (b *Batch) withConn(ctx context.Context, fn func() error) error { defer b.st.conn.contextWatcher.Unwatch() } - return fn() + return operationerror.WithRetry(normalizeTimeoutError(ctx, fn()), false) } func (b *Batch) receiveCompletion(ctx context.Context) (*BatchCompletion, error) { - for { - msg, err := b.st.conn.receiveMessage() - if err != nil { - b.st.conn.asyncClose() - return nil, normalizeTimeoutError(ctx, err) - } + msg, err := b.st.conn.receiveMessage() + if err != nil { + b.st.conn.asyncCloseAfterRead(err) + return nil, normalizeTimeoutError(ctx, err) + } - switch m := msg.(type) { - case *fbproto.OpBatchCompletion: - return b.convertCompletion(m), nil - case *fbproto.OpResponse: - continue - case *fbproto.ErrorResponse: - return nil, ErrorResponseToFbError(m) - default: - return nil, fmt.Errorf("received unexpected batch response %T", msg) + switch m := msg.(type) { + case *fbproto.OpBatchCompletion: + if m.StatementHandle != b.st.Description.StatementHandle { + b.st.conn.asyncClose() + return nil, fmt.Errorf("batch response statement handle %d, expected %d", m.StatementHandle, b.st.Description.StatementHandle) } + return b.convertCompletion(m), nil + case *fbproto.ErrorResponse: + return nil, ErrorResponseToFbError(m) + default: + b.st.conn.asyncClose() + return nil, fmt.Errorf("received unexpected batch response %T", msg) } } func (b *Batch) resolveTx(ctx context.Context, tx *FBTransaction) (*FBTransaction, error) { if tx != nil { + if tx.Connection != b.st.conn { + return nil, fmt.Errorf("batch transaction belongs to another connection") + } if tx.Closed { return nil, ErrTransactionClosed } @@ -406,6 +590,7 @@ func (b *Batch) convertCompletion(c *fbproto.OpBatchCompletion) *BatchCompletion func (b *Batch) markClosed() { b.state = batchStateClosed + b.bufferState = batchBufferEmpty if b.st != nil && b.st.activeBatch == b { b.st.activeBatch = nil } diff --git a/fbconn/fbconn.go b/fbconn/fbconn.go index bdce366..6318b23 100644 --- a/fbconn/fbconn.go +++ b/fbconn/fbconn.go @@ -1538,6 +1538,11 @@ type QueryStatus struct { rowsAffected int64 } +// NewQueryStatus creates a status with the statement type and affected row count. +func NewQueryStatus(statementType int, rowsAffected int64) QueryStatus { + return QueryStatus{statementType: statementType, rowsAffected: rowsAffected} +} + // RowsAffected returns the number of rows affected func (ct QueryStatus) RowsAffected() int64 { return ct.rowsAffected diff --git a/fbconn/fbconn_private_test.go b/fbconn/fbconn_private_test.go index 986bd6a..63d728b 100644 --- a/fbconn/fbconn_private_test.go +++ b/fbconn/fbconn_private_test.go @@ -1,9 +1,16 @@ package fbconn import ( + "bytes" "context" + "encoding/binary" "fmt" + "io" + "math" + "net" + "strconv" "testing" + "time" "github.com/firebirdsql/fbx/fbproto" "github.com/stretchr/testify/assert" @@ -37,21 +44,555 @@ func TestCommandTag(t *testing.T) { } } +func TestBatchInvalidConfigDoesNotClaimStatement(t *testing.T) { + type testCase struct { + name string + cfg BatchConfig + err string + } + tests := []testCase{ + {"negative buffer", BatchConfig{BufferBytes: -2}, "batch buffer size"}, + {"negative details", BatchConfig{DetailedErrs: -2}, "batch detailed error limit"}, + {"invalid blob policy", BatchConfig{BlobPolicy: 4}, "batch blob policy"}, + } + if strconv.IntSize == 64 { + overflow := int64(math.MaxInt32) + 1 + tests = append(tests, + testCase{"buffer overflow", BatchConfig{BufferBytes: int(overflow)}, "batch buffer size"}, + testCase{"details overflow", BatchConfig{DetailedErrs: int(overflow)}, "batch detailed error limit"}, + ) + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := &Statement{} + b := st.NewBatch(tt.cfg) + ctx := context.Background() + require.ErrorContains(t, b.Create(ctx), tt.err) + require.ErrorContains(t, b.Send(ctx, [][]byte{{0, 0, 0, 1}}), tt.err) + completion, err := b.Execute(ctx, nil) + require.ErrorContains(t, err, tt.err) + require.Nil(t, completion) + require.Nil(t, st.activeBatch) + + valid := st.NewBatch(BatchConfig{}) + require.NoError(t, valid.err) + require.NoError(t, b.Close(ctx)) + require.Same(t, valid, st.activeBatch) + }) + } +} + +func TestBatchConfigParameterBuffer(t *testing.T) { + tests := []struct { + name string + cfg BatchConfig + want map[byte][]byte + }{ + {"server defaults", BatchConfig{BufferBytes: -1, DetailedErrs: -1}, map[byte][]byte{ + fbproto.BatchTagBlobPolicy: {0}, + }}, + {"explicit zeros", BatchConfig{}, map[byte][]byte{ + fbproto.BatchTagBufferBytes: {0, 0, 0, 0}, + fbproto.BatchTagDetailedErrors: {0, 0, 0, 0}, + fbproto.BatchTagBlobPolicy: {0}, + }}, + {"maximum wire values", BatchConfig{MultiError: true, RecordCounts: true, BufferBytes: math.MaxInt32, DetailedErrs: math.MaxInt32, BlobPolicy: fbproto.BatchBlobStream}, map[byte][]byte{ + fbproto.BatchTagMultiError: {1, 0, 0, 0}, + fbproto.BatchTagRecordCounts: {1, 0, 0, 0}, + fbproto.BatchTagBufferBytes: {0xff, 0xff, 0xff, 0x7f}, + fbproto.BatchTagDetailedErrors: {0xff, 0xff, 0xff, 0x7f}, + fbproto.BatchTagBlobPolicy: {fbproto.BatchBlobStream}, + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := (&Statement{}).NewBatch(tt.cfg) + require.NoError(t, b.err) + r := fbproto.NewXPBReader(b.cfg.parameterBuffer()) + ok, version := r.Next() + require.True(t, ok) + require.EqualValues(t, fbproto.BatchVersion1, version) + got := make(map[byte][]byte) + for ok, tag := r.Next(); ok; ok, tag = r.Next() { + value, ok := r.GetBytes(int(r.GetInt32())) + require.True(t, ok) + got[tag] = value + } + require.Equal(t, tt.want, got) + }) + } +} + +func TestBatchCompletionAffected(t *testing.T) { + tests := []struct { + name string + completion *BatchCompletion + want int64 + }{ + {"nil completion", nil, 0}, + {"empty batch", &BatchCompletion{}, 0}, + {"multiple affected rows per message", &BatchCompletion{TotalRecords: 2, UpdateCounts: []int64{0, 7}}, 7}, + {"no affected rows", &BatchCompletion{TotalRecords: 2, UpdateCounts: []int64{0, 0}}, 0}, + {"counts disabled", &BatchCompletion{TotalRecords: 2}, -1}, + {"incomplete counts", &BatchCompletion{TotalRecords: 2, UpdateCounts: []int64{7}}, -1}, + {"failed message", &BatchCompletion{TotalRecords: 3, UpdateCounts: []int64{1, fbproto.BatchExecuteFailed, 2}}, 3}, + {"success without count", &BatchCompletion{TotalRecords: 2, UpdateCounts: []int64{7, fbproto.BatchSuccessNoInfo}}, -1}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, tt.completion.Affected()) + }) + } +} + +func TestBatchInfoParsing(t *testing.T) { + var data []byte + for i, value := range []uint32{65536, 24, 64, 4, 16} { + data = append(data, byte(fbproto.BatchInfoBufferBytes+i), 4, 0) + data = binary.LittleEndian.AppendUint32(data, value) + } + data = append(data, fbproto.Isc_info_end) + want := BatchInfo{BufferBytes: 65536, DataBytes: 24, BlobBytes: 64, BlobAlignment: 4, BlobHeaderBytes: 16} + info, err := parseBatchInfo(data) + require.NoError(t, err) + require.Equal(t, want, info) + + withoutBlobs := append(append([]byte(nil), data[:14]...), data[21:]...) + info, err = parseBatchInfo(withoutBlobs) + require.NoError(t, err) + want.BlobBytes = 0 + require.Equal(t, want, info) + + for n := 0; n < len(data); n++ { + info, err = parseBatchInfo(data[:n]) + require.Error(t, err, "prefix of %d bytes", n) + require.Zero(t, info) + } + for _, malformed := range [][]byte{ + {fbproto.Isc_info_truncated}, {fbproto.Isc_info_error}, {99}, + data[7:], // missing buffer capacity + {fbproto.BatchInfoBufferBytes, 9, 0}, // invalid integer length + {fbproto.BatchInfoBufferBytes, 4, 0, 0xff, 0xff, 0xff, 0xff}, + } { + info, err = parseBatchInfo(malformed) + require.Error(t, err) + require.Zero(t, info) + } +} + +func TestBatchInfoErrorsPreserveQueue(t *testing.T) { + for _, mode := range []string{"server error", "malformed info", "lost response", "unexpected response", "write failure", "canceled", "unsupported protocol"} { + t.Run(mode, func(t *testing.T) { + var input bytes.Buffer + w := fbproto.NewXDRWriter(&input) + switch mode { + case "server error": + require.NoError(t, retryTestResponse(w, fbproto.Isc_deadlock)) + case "malformed info": + require.NoError(t, writeProbeResponseData(w, 0, []byte{fbproto.Isc_info_truncated})) + case "unexpected response": + require.NoError(t, w.WriteInt32(fbproto.Op_sql_response)) + require.NoError(t, w.WriteInt32(0)) + } + b, transport := newBatchProtocolTest(t, &input) + b.bufferState = batchBufferPending + ctx := context.Background() + if mode == "canceled" { + var cancel context.CancelFunc + ctx, cancel = context.WithCancel(ctx) + cancel() + } + if mode == "unsupported protocol" { + b.st.conn.protocolVersion = fbproto.PROTOCOL_VERSION16 + b.state = batchStateInit + } + if mode == "write failure" { + transport.writeErr = io.ErrClosedPipe + } + info, err := b.Info(ctx) + require.Error(t, err) + require.Zero(t, info) + require.Equal(t, batchBufferPending, b.bufferState) + require.Equal(t, mode == "lost response" || mode == "unexpected response" || mode == "write failure", b.st.conn.IsClosed()) + require.Equal(t, mode == "canceled", SafeToRetry(err)) + if mode == "canceled" { + require.ErrorIs(t, err, context.Canceled) + } + if mode == "unsupported protocol" || mode == "canceled" { + require.Zero(t, transport.written.Len(), "must not create or query a batch") + } else if !b.st.conn.IsClosed() { + require.NoError(t, writeProbeResponse(w, 0)) + require.NoError(t, writeProbeResponse(w, 0)) + require.NoError(t, b.Cancel(context.Background())) + require.False(t, b.HasPending()) + } + }) + } +} + func TestBatchCloseCanceledContextKeepsBatchOpen(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() + for _, deadline := range []bool{false, true} { + t.Run(fmt.Sprintf("deadline=%t", deadline), func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + if deadline { + cancel() + ctx, cancel = context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + } + cancel() - conn := &FbConn{status: connStatusIdle} - st := &Statement{conn: conn} - b := &Batch{st: st, state: batchStateCreated} - st.activeBatch = b + conn := &FbConn{status: connStatusIdle} + st := &Statement{conn: conn} + b := &Batch{st: st, state: batchStateCreated} + st.activeBatch = b - err := b.Close(ctx) + err := b.Close(ctx) - require.Error(t, err) - require.False(t, b.isClosed()) - require.Same(t, b, st.activeBatch) - require.False(t, conn.IsClosed()) + require.ErrorIs(t, err, ctx.Err()) + require.True(t, SafeToRetry(err)) + require.False(t, b.isClosed()) + require.Same(t, b, st.activeBatch) + require.False(t, conn.IsClosed()) + }) + } +} + +func newBatchProtocolTest(t *testing.T, input io.Reader) (*Batch, *prepareCleanupTestConn) { + t.Helper() + transport := &prepareCleanupTestConn{} + conn := newDetachTestConn(transport) + conn.protocolVersion = fbproto.PROTOCOL_VERSION17 + encoding, err := fbproto.NewEncoding("UTF8") + require.NoError(t, err) + conn.client = fbproto.NewClient(input, conn.writer, encoding) + st := &Statement{conn: conn, Description: &StatementDescription{ + StatementHandle: 42, + StatementInfo: &StatementInfo{ + ParametersDescriptors: []fbproto.FieldDescriptor{{Type: fbproto.SQL_TYPE_LONG, Length: 4}}, + }, + }} + b := st.NewBatch(BatchConfig{RecordCounts: true}) + b.state = batchStateCreated + t.Cleanup(func() { + conn.asyncClose() + waitCleanupDone(t, conn) + }) + return b, transport +} + +func TestBatchSyncResponses(t *testing.T) { + for _, mode := range []string{"server error", "sync error", "broken response"} { + t.Run(mode, func(t *testing.T) { + var input bytes.Buffer + w := fbproto.NewXDRWriter(&input) + broken := mode == "broken response" + if broken { + require.NoError(t, w.WriteInt32(fbproto.Op_sql_response)) + require.NoError(t, w.WriteInt32(0)) + } else if mode == "server error" { + require.NoError(t, retryTestResponse(w, fbproto.Isc_deadlock)) + } else { + require.NoError(t, writeProbeResponse(w, 0)) + } + if mode == "sync error" { + require.NoError(t, retryTestResponse(w, fbproto.Isc_deadlock)) + } else { + require.NoError(t, writeProbeResponse(w, 0)) + } + require.NoError(t, writeProbeResponse(w, 77)) + b, _ := newBatchProtocolTest(t, &input) + conn := b.st.conn + var resets []int + h := &CancelRequestContextWatcherHandler{Conn: conn, sendCancelOperationFunc: func(_ context.Context, kind int) error { + resets = append(resets, kind) + return nil + }} + h.pendingCancelReset.Store(true) + conn.contextHandler = h + + err := b.Send(context.Background(), [][]byte{{0, 0, 0, 1}}) + require.Error(t, err) + require.True(t, b.NeedsReset()) + require.True(t, b.HasPending()) + require.False(t, SafeToRetry(err)) + require.Equal(t, []int{fbproto.Fb_cancel_disable, fbproto.Fb_cancel_enable}, resets) + require.Equal(t, broken, conn.IsClosed()) + if broken { + require.Equal(t, 2*24, input.Len(), "do not read another response after closing the connection") + return + } + var fbErr *FbError + require.ErrorAs(t, err, &fbErr) + require.Contains(t, fbErr.GdsCodes, fbproto.Isc_deadlock) + msg, err := conn.receiveMessage() + require.NoError(t, err) + require.Equal(t, 77, msg.(*fbproto.OpResponse).Object, "the sync response must be consumed") + + require.NoError(t, retryTestResponse(w, fbproto.Isc_cancelled)) + require.NoError(t, writeProbeResponse(w, 0)) + require.Error(t, b.Cancel(context.Background())) + require.True(t, b.NeedsReset()) + require.Same(t, b, b.st.activeBatch, "failed cleanup must retain the handle") + require.NoError(t, writeProbeResponse(w, 0)) + require.NoError(t, writeProbeResponse(w, 0)) + require.NoError(t, b.Cancel(context.Background())) + require.False(t, b.HasPending()) + require.False(t, b.NeedsReset()) + require.False(t, b.isClosed()) + }) + } +} + +func TestBatchWriteFailureDisallowsRetry(t *testing.T) { + t.Run("write error", func(t *testing.T) { + b, transport := newBatchProtocolTest(t, bytes.NewReader(nil)) + transport.writeErr = &fbconnError{msg: "write failed", safeToRetry: true} + + err := b.Send(context.Background(), [][]byte{{0, 0, 0, 1}}) + + require.ErrorIs(t, err, transport.writeErr) + require.False(t, SafeToRetry(err)) + require.True(t, b.st.conn.IsClosed()) + b.st.conn.writeMu.Lock() + failed := b.st.conn.writeFailed + b.st.conn.writeMu.Unlock() + require.True(t, failed) + }) + + t.Run("canceled during write", func(t *testing.T) { + client, server := net.Pipe() + defer func() { _ = server.Close() }() + conn := newDetachTestConn(client) + conn.protocolVersion = fbproto.PROTOCOL_VERSION17 + t.Cleanup(func() { + conn.asyncClose() + waitCleanupDone(t, conn) + }) + st := &Statement{conn: conn, Description: &StatementDescription{StatementHandle: 42}} + b := st.NewBatch(BatchConfig{}) + b.state = batchStateCreated + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan error, 1) + go func() { done <- b.Send(ctx, [][]byte{{0, 0, 0, 1}, {0, 0, 0, 2}}) }() + + // Read the header and first row, leaving the second row's write blocked. + require.NoError(t, server.SetReadDeadline(time.Now().Add(3*time.Second))) + r := fbproto.NewXDRReader(server) + for _, want := range []int{fbproto.Op_batch_msg, 42, 2, 1} { + got, err := r.ReadInt32() + require.NoError(t, err) + require.Equal(t, want, got) + } + cancel() + + select { + case err := <-done: + require.ErrorIs(t, err, context.Canceled) + require.False(t, SafeToRetry(err)) + case <-time.After(3 * time.Second): + t.Fatal("batch write did not stop after cancellation") + } + require.True(t, conn.IsClosed()) + conn.writeMu.Lock() + failed := conn.writeFailed + conn.writeMu.Unlock() + require.True(t, failed) + }) +} + +func TestBatchExecuteLostCommitReplyPreservesCompletion(t *testing.T) { + var input bytes.Buffer + w := fbproto.NewXDRWriter(&input) + // Execution succeeded for both rows, but there is no reply to the following commit. + for _, n := range []int{fbproto.Op_batch_cs, 42, 2, 2, 0, 0, 1, 1} { + require.NoError(t, w.WriteInt32(n)) + } + b, transport := newBatchProtocolTest(t, &input) + b.bufferState = batchBufferPending + conn := b.st.conn + conn.config.Autocommit = true + tx := &FBTransaction{Connection: conn, Handle: 42} + conn.internalTransaction = tx + conn.activeTransactions = map[*FBTransaction]struct{}{tx: {}} + + completion, err := b.Execute(context.Background(), nil) + + var fbErr *FbError + require.ErrorAs(t, err, &fbErr) + require.Equal(t, "08006", fbErr.Code) + require.False(t, SafeToRetry(err)) + require.True(t, conn.IsClosed()) + require.NotNil(t, completion) + require.EqualValues(t, 2, completion.TotalRecords) + require.Equal(t, []int64{1, 1}, completion.UpdateCounts) + require.Empty(t, completion.Errors) + require.False(t, b.HasPending(), "the completion confirms that the server consumed the queue") + + r := fbproto.NewXDRReader(&transport.written) + for _, want := range []int{fbproto.Op_batch_exec, 42, 42, fbproto.Op_commit, 42} { + got, err := r.ReadInt32() + require.NoError(t, err) + require.Equal(t, want, got) + } + require.Zero(t, transport.written.Len(), "do not send rollback after losing the connection") +} + +func TestBatchAcknowledgementSurvivesSyncError(t *testing.T) { + for _, create := range []bool{true, false} { + t.Run(fmt.Sprintf("create=%t", create), func(t *testing.T) { + var input bytes.Buffer + w := fbproto.NewXDRWriter(&input) + require.NoError(t, writeProbeResponse(w, 0)) + require.NoError(t, retryTestResponse(w, fbproto.Isc_cancelled)) + b, _ := newBatchProtocolTest(t, &input) + var err error + if create { + b.state = batchStateInit + err = b.Create(context.Background()) + require.Equal(t, batchStateCreated, b.state, "Close must release the acknowledged batch") + } else { + err = b.Close(context.Background()) + require.True(t, b.isClosed(), "release already succeeded") + require.Nil(t, b.st.activeBatch) + } + require.Error(t, err) + require.False(t, b.st.conn.IsClosed()) + require.Zero(t, input.Len()) + }) + } +} + +type batchCancelReader struct { + io.Reader + cancel context.CancelFunc +} + +func (r *batchCancelReader) Read(p []byte) (int, error) { + if r.cancel != nil { + r.cancel() + r.cancel = nil + } + return r.Reader.Read(p) +} + +func TestBatchExecuteCancellation(t *testing.T) { + for _, explicit := range []bool{false, true} { + for _, rowError := range []bool{false, true} { + t.Run(fmt.Sprintf("explicit=%t/rowError=%t", explicit, rowError), func(t *testing.T) { + var input bytes.Buffer + w := fbproto.NewXDRWriter(&input) + if rowError { + for _, n := range []int{fbproto.Op_batch_cs, 42, 2, 2, 1, 0, 1, -1, 1, fbproto.Isc_arg_gds, fbproto.Isc_cancelled, fbproto.Isc_arg_end} { + require.NoError(t, w.WriteInt32(n)) + } + } else { + require.NoError(t, retryTestResponse(w, fbproto.Isc_cancelled)) + } + if !explicit { + require.NoError(t, writeProbeResponse(w, 0)) + } + if !rowError { + require.NoError(t, writeProbeResponse(w, 0)) + require.NoError(t, writeProbeResponse(w, 0)) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + b, transport := newBatchProtocolTest(t, &batchCancelReader{Reader: &input, cancel: cancel}) + conn := b.st.conn + conn.config.Autocommit = true + conn.explicitTransaction = explicit + tx := &FBTransaction{Connection: conn, Handle: 42} + conn.internalTransaction = tx + conn.activeTransactions = map[*FBTransaction]struct{}{tx: {}} + + completion, err := b.Execute(ctx, tx) + + require.ErrorIs(t, err, context.Canceled) + require.False(t, SafeToRetry(err)) + require.False(t, conn.IsClosed()) + require.Equal(t, explicit, conn.ActiveTx()) + require.Equal(t, !rowError, b.NeedsReset(), "only a completion confirms that execution consumed the queue") + require.Equal(t, !rowError, b.HasPending()) + if rowError { + require.NotNil(t, completion) + require.Equal(t, []int64{1, -1}, completion.UpdateCounts) + require.Len(t, completion.Errors, 1) + } else { + require.Nil(t, completion) + var fbErr *FbError + require.ErrorAs(t, err, &fbErr) + require.Contains(t, fbErr.GdsCodes, fbproto.Isc_cancelled) + } + if explicit { + require.Equal(t, 12, transport.written.Len(), "must not roll back a caller-owned transaction") + } else { + require.Equal(t, 20, transport.written.Len()) + require.EqualValues(t, fbproto.Op_rollback, binary.BigEndian.Uint32(transport.written.Bytes()[12:])) + } + if !rowError { + written := transport.written.Len() + require.ErrorContains(t, b.Send(context.Background(), [][]byte{{0, 0, 0, 1}}), "requires Cancel") + require.Equal(t, written, transport.written.Len(), "do not append to an uncertain server queue") + require.NoError(t, b.Cancel(context.Background())) + require.False(t, b.NeedsReset()) + require.False(t, b.HasPending()) + require.False(t, b.isClosed()) + } + require.Zero(t, input.Len()) + }) + } + } +} + +func TestBatchPendingSurvivesCanceledOperation(t *testing.T) { + for _, execute := range []bool{false, true} { + t.Run(fmt.Sprintf("execute=%t", execute), func(t *testing.T) { + var input bytes.Buffer + w := fbproto.NewXDRWriter(&input) + require.NoError(t, writeProbeResponse(w, 0)) + require.NoError(t, writeProbeResponse(w, 0)) + b, transport := newBatchProtocolTest(t, &input) + rows := [][]byte{{0, 0, 0, 1}} + require.NoError(t, b.Send(context.Background(), rows)) + written := transport.written.Len() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + var err error + if execute { + _, err = b.Execute(ctx, &FBTransaction{Connection: b.st.conn, Handle: 42}) + } else { + err = b.Send(ctx, rows) + } + require.ErrorIs(t, err, context.Canceled) + require.True(t, SafeToRetry(err)) + require.True(t, b.HasPending()) + require.False(t, b.NeedsReset()) + require.Equal(t, written, transport.written.Len()) + }) + } +} + +func TestBatchExecuteRejectsInvalidReply(t *testing.T) { + for _, reply := range [][]int{ + {fbproto.Op_batch_cs, 43, 0, 0, 0, 0}, + {fbproto.Op_batch_cs, 42, 1, -1, 0, 0}, + {fbproto.Op_batch_cs, 42, 1}, + {fbproto.Op_response, 0, 0, 0, 0, fbproto.Isc_arg_end}, + } { + t.Run(fmt.Sprint(reply), func(t *testing.T) { + var input bytes.Buffer + for _, n := range reply { + require.NoError(t, fbproto.NewXDRWriter(&input).WriteInt32(n)) + } + b, _ := newBatchProtocolTest(t, &input) + completion, err := b.Execute(context.Background(), &FBTransaction{Connection: b.st.conn, Handle: 42}) + require.Nil(t, completion) + require.Error(t, err) + require.False(t, SafeToRetry(err)) + require.True(t, b.st.conn.IsClosed()) + }) + } } func TestStatementDeallocateCanceledContextKeepsStatementOpen(t *testing.T) { diff --git a/fbconn/fbconn_test.go b/fbconn/fbconn_test.go index f64d529..37b521a 100644 --- a/fbconn/fbconn_test.go +++ b/fbconn/fbconn_test.go @@ -1406,211 +1406,6 @@ func TestConnExecPreparedEmptySQL(t *testing.T) { ensureConnValid(t, fbConn) } -func TestConnExecBatch(t *testing.T) { - //t.Parallel() - // - //ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - //defer cancel() - // - //fbConn, err := fbconn.Connect(ctx, os.Getenv("FBX_TEST_DATABASE")) - //require.NoError(t, err) - //defer closeConn(t, fbConn) - // - //_, err = fbConn.Prepare(ctx, "ps1", "select $1::text", nil) - //require.NoError(t, err) - // - //batch := &fbconn.Batch{} - // - //batch.ExecParams("select $1::text", [][]byte{[]byte("ExecParams 1")}, nil, nil, nil) - //batch.ExecPrepared("ps1", [][]byte{[]byte("ExecPrepared 1")}, nil, nil) - //batch.ExecParams("select $1::text", [][]byte{[]byte("ExecParams 2")}, nil, nil, nil) - //results, err := fbConn.ExecBatch(ctx, batch).ReadAll() - //require.NoError(t, err) - //require.Len(t, results, 3) - // - //require.Len(t, results[0].Rows, 1) - //require.Equal(t, "ExecParams 1", string(results[0].Rows[0][0])) - //assert.Equal(t, "SELECT 1", results[0].QueryStatus.String()) - // - //require.Len(t, results[1].Rows, 1) - //require.Equal(t, "ExecPrepared 1", string(results[1].Rows[0][0])) - //assert.Equal(t, "SELECT 1", results[1].QueryStatus.String()) - // - //require.Len(t, results[2].Rows, 1) - //require.Equal(t, "ExecParams 2", string(results[2].Rows[0][0])) - //assert.Equal(t, "SELECT 1", results[2].QueryStatus.String()) -} - -//func TestConnExecBatchWriteError(t *testing.T) { -// t.Skip("need implementation") -// t.Parallel() -// -// ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) -// defer cancel() -// -// config, err := fbconn.ParseConfig(os.Getenv("FBX_TEST_DATABASE")) -// require.NoError(t, err) -// -// var mockConn mockConnection -// writeLatency := 0 * time.Second -// config.DialFunc = func(ctx context.Context, network, address string) (net.Conn, error) { -// conn, err := net.Dial(network, address) -// mockConn = mockConnection{conn, &writeLatency} -// return mockConn, err -// } -// -// fbConn, err := fbconn.ConnectConfig(ctx, config) -// require.NoError(t, err) -// defer closeConn(t, fbConn) -// -// batch := &fbconn.Batch{} -// fbConn.Conn() -// -// ctx2, cancel2 := context.WithTimeout(context.Background(), 1*time.Second) -// defer cancel2() -// -// batch.ExecParams("select $1::text", [][]byte{[]byte("ExecParams 1")}, nil, nil, nil) -// writeLatency = 2 * time.Second -// mrr := fbConn.ExecBatch(ctx2, batch) -// err = mrr.Close() -// require.Error(t, err) -// assert.ErrorIs(t, err, context.DeadlineExceeded) -// require.True(t, fbConn.IsClosed()) -//} - -//func TestConnExecBatchDeferredError(t *testing.T) { -// t.Skip("need batch implementation") -// t.Parallel() -// -// ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) -// defer cancel() -// -// fbConn, err := fbconn.Connect(ctx, os.Getenv("FBX_TEST_DATABASE")) -// require.NoError(t, err) -// defer closeConn(t, fbConn) -// -// if fbConn.ParameterStatus("crdb_version") != "" { -// t.Skip("Server does not support deferred constraint") -// } -// -// setupSQL := `create temporary table t ( -// id text primary key, -// n int not null, -// unique (n) deferrable initially deferred -// ); -// -// insert into t (id, n) values ('a', 1), ('b', 2), ('c', 3);` -// -// res := fbConn.Exec(ctx, setupSQL).Read() -// require.NoError(t, res.Err) -// -// batch := &fbconn.Batch{} -// -// batch.ExecParams(`update t set n=n+1 where id='b' returning *`, nil, nil, nil, nil) -// _, err = fbConn.ExecBatch(ctx, batch).ReadAll() -// require.NotNil(t, err) -// var fbErr *fbconn.FbError -// require.True(t, errors.As(err, &fbErr)) -// require.Equal(t, "23505", fbErr.Code) -// -// ensureConnValid(t, fbConn) -//} - -//func TestConnExecBatchPrecanceled(t *testing.T) { -//t.Parallel() -// -//ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) -//defer cancel() -// -//fbConn, err := fbconn.Connect(ctx, os.Getenv("FBX_TEST_DATABASE")) -//require.NoError(t, err) -//defer closeConn(t, fbConn) -// -//_, err = fbConn.PrepareTx(ctx, "ps1", "select $1::text", nil) -//require.NoError(t, err) -// -//batch := &fbconn.Batch{} -// -//batch.ExecParams("select $1::text", [][]byte{[]byte("ExecParams 1")}, nil, nil, nil) -//batch.ExecPrepared("ps1", [][]byte{[]byte("ExecPrepared 1")}, nil, nil) -//batch.ExecParams("select $1::text", [][]byte{[]byte("ExecParams 2")}, nil, nil, nil) -// -//cancel() -//_, err = fbConn.ExecBatch(ctx, batch).ReadAll() -//require.Error(t, err) -//assert.True(t, errors.Is(err, context.Canceled)) -//assert.True(t, fbconn.SafeToRetry(err)) -// -//ensureConnValid(t, fbConn) -//} - -// Without concurrent reading and writing large batches can deadlock. -// -//func TestConnExecBatchHuge(t *testing.T) { -// if testing.Short() { -// t.Skip("skipping test in short mode.") -// } -// -// t.Parallel() -// -// ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) -// defer cancel() -// -// fbConn, err := fbconn.Connect(ctx, os.Getenv("FBX_TEST_DATABASE")) -// require.NoError(t, err) -// defer closeConn(t, fbConn) -// -// batch := &fbconn.Batch{} -// -// queryCount := 100000 -// args := make([]string, queryCount) -// -// for i := range args { -// args[i] = strconv.Itoa(i) -// batch.ExecParams("select $1::text", [][]byte{[]byte(args[i])}, nil, nil, nil) -// } -// -// results, err := fbConn.ExecBatch(ctx, batch).ReadAll() -// require.NoError(t, err) -// require.Len(t, results, queryCount) -// -// for i := range args { -// require.Len(t, results[i].Rows, 1) -// require.Equal(t, args[i], string(results[i].Rows[0][0])) -// assert.Equal(t, "SELECT 1", results[i].QueryStatus.String()) -// } -//} - -//func TestConnExecBatchImplicitTransaction(t *testing.T) { -//t.Parallel() -// -//ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) -//defer cancel() -// -//fbConn, err := fbconn.Connect(ctx, os.Getenv("FBX_TEST_DATABASE")) -//require.NoError(t, err) -//defer closeConn(t, fbConn) -// -//if fbConn.ParameterStatus("crdb_version") != "" { -// t.Skip() -//} -// -//_, err = fbConn.Exec(ctx, "create temporary table t(id int)").ReadAll() -//require.NoError(t, err) -// -//batch := &fbconn.Batch{} -// -//batch.ExecParams("insert into t(id) values(1)", nil, nil, nil, nil) -//batch.ExecParams("insert into t(id) values(2)", nil, nil, nil, nil) -//batch.ExecParams("insert into t(id) values(3)", nil, nil, nil, nil) -//batch.ExecParams("select 1/0", nil, nil, nil, nil) -//_, err = fbConn.ExecBatch(ctx, batch).ReadAll() -//require.Error(t, err) -// -//result := fbConn.ExecParams(ctx, "select count(*) from t", nil, nil, nil, nil).Read() -//require.Equal(t, "0", string(result.Rows[0][0])) -//} - func TestConnLocking(t *testing.T) { t.Parallel() @@ -1636,162 +1431,6 @@ func TestConnLocking(t *testing.T) { ensureConnValid(t, fbConn) } -func TestConnOnNotice(t *testing.T) { - // t.Skip("need events implementation") - // t.Parallel() - - // ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - // defer cancel() - - // config, err := fbconn.ParseConfig(os.Getenv("FBX_TEST_DATABASE")) - // require.NoError(t, err) - - // var notice *fbconn.Notice - // config.OnNotice = func(c *fbconn.FbConn, n *fbconn.Notice) { - // notice = n - // } - // config.RuntimeParams["client_min_messages"] = "notice" // Ensure we only get the message we expect. - - // fbConn, err := fbconn.ConnectConfig(ctx, config) - // require.NoError(t, err) - // defer closeConn(t, fbConn) - - // if fbConn.ParameterStatus("crdb_version") != "" { - // t.Skip("") - // } - - // rr := fbConn.Exec(ctx, `do $$ - // begin - // raise notice 'hello, world'; - // end$$;`) - // res := rr.Read() - // require.NoError(t, res.Err) - // //assert.Equal(t, "NOTICE", notice.SeverityUnlocalized) - // assert.Equal(t, "hello, world", notice.Message) - - // ensureConnValid(t, fbConn) -} - -func TestConnOnNotification(t *testing.T) { - //t.Parallel() - // - //ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - //defer cancel() - // - //config, err := fbconn.ParseConfig(os.Getenv("FBX_TEST_DATABASE")) - //require.NoError(t, err) - // - //var msg string - //config.OnNotification = func(c *fbconn.FbConn, n *fbconn.Notification) { - // msg = n.Payload - //} - // - //fbConn, err := fbconn.ConnectConfig(ctx, config) - //require.NoError(t, err) - //defer closeConn(t, fbConn) - // - //if fbConn.ParameterStatus("crdb_version") != "" { - // t.Skip("Server does not support LISTEN / NOTIFY") - //} - // - //_, err = fbConn.Exec(ctx, "listen foo").ReadAll() - //require.NoError(t, err) - // - //notifier, err := fbconn.ConnectConfig(ctx, config) - //require.NoError(t, err) - //defer closeConn(t, notifier) - //_, err = notifier.Exec(ctx, "notify foo, 'bar'").ReadAll() - //require.NoError(t, err) - // - //_, err = fbConn.Exec(ctx, "select 1").ReadAll() - //require.NoError(t, err) - // - //assert.Equal(t, "bar", msg) - // - //ensureConnValid(t, fbConn) -} - -func TestConnWaitForNotification(t *testing.T) { - //t.Parallel() - // - //ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - //defer cancel() - // - //config, err := fbconn.ParseConfig(os.Getenv("FBX_TEST_DATABASE")) - //require.NoError(t, err) - // - //var msg string - //config.OnNotification = func(c *fbconn.FbConn, n *fbconn.Notification) { - // msg = n.Payload - //} - // - //fbConn, err := fbconn.ConnectConfig(ctx, config) - //require.NoError(t, err) - //defer closeConn(t, fbConn) - // - //if fbConn.ParameterStatus("crdb_version") != "" { - // t.Skip("Server does not support LISTEN / NOTIFY") - //} - // - //_, err = fbConn.Exec(ctx, "listen foo").ReadAll() - //require.NoError(t, err) - // - //notifier, err := fbconn.ConnectConfig(ctx, config) - //require.NoError(t, err) - //defer closeConn(t, notifier) - //_, err = notifier.Exec(ctx, "notify foo, 'bar'").ReadAll() - //require.NoError(t, err) - // - //err = fbConn.WaitForNotification(ctx) - //require.NoError(t, err) - // - //assert.Equal(t, "bar", msg) - // - //ensureConnValid(t, fbConn) -} - -func TestConnWaitForNotificationPrecanceled(t *testing.T) { - //t.Parallel() - // - //ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - //defer cancel() - // - //config, err := fbconn.ParseConfig(os.Getenv("FBX_TEST_DATABASE")) - //require.NoError(t, err) - // - //fbConn, err := fbconn.ConnectConfig(ctx, config) - //require.NoError(t, err) - //defer closeConn(t, fbConn) - // - //cancel() - //err = fbConn.WaitForNotification(ctx) - //require.ErrorIs(t, err, context.Canceled) - // - //ensureConnValid(t, fbConn) -} - -func TestConnWaitForNotificationTimeout(t *testing.T) { - //t.Parallel() - // - //ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - //defer cancel() - // - //config, err := fbconn.ParseConfig(os.Getenv("FBX_TEST_DATABASE")) - //require.NoError(t, err) - // - //fbConn, err := fbconn.ConnectConfig(ctx, config) - //require.NoError(t, err) - //defer closeConn(t, fbConn) - // - //ctx, cancel = context.WithTimeout(ctx, 5*time.Millisecond) - //err = fbConn.WaitForNotification(ctx) - //cancel() - //assert.True(t, fbconn.Timeout(err)) - //assert.ErrorIs(t, err, context.DeadlineExceeded) - // - //ensureConnValid(t, fbConn) -} - func TestConnEscapeString(t *testing.T) { t.Parallel() diff --git a/fbconn/statement.go b/fbconn/statement.go index 5dbc7db..6b0ddf0 100644 --- a/fbconn/statement.go +++ b/fbconn/statement.go @@ -93,8 +93,6 @@ func (sd *StatementDescription) EncodeBatchRow(values [][]byte) ([]byte, error) return nil, err } - // row := fbproto.AppendRowData(nil, b.sd.StatementInfo.ParametersDescriptors, b.eqb.ParamValues) - return buf.Bytes(), nil } @@ -127,8 +125,12 @@ func (st *Statement) SetInlineBlobSize(size int) { st.conn.inlineBlobSize = size } +// NewBatch creates a local batch for st without performing network I/O. +// [Batch.Create], also called by Send and Execute, creates the server batch. +// A statement may have only one open batch; configuration and initialization +// errors are reported by Create without sending a request. func (st *Statement) NewBatch(cfg BatchConfig) *Batch { - b := &Batch{st: st, cfg: cfg.normalize()} + b := &Batch{st: st, cfg: cfg} if st.closed { b.err = fmt.Errorf("statement is closed") return b @@ -137,6 +139,10 @@ func (st *Statement) NewBatch(cfg BatchConfig) *Batch { b.err = fmt.Errorf("batch is already opened for this statement") return b } + if err := cfg.validate(); err != nil { + b.err = err + return b + } st.activeBatch = b return b } diff --git a/fbproto/op_batch_completion.go b/fbproto/op_batch_completion.go index 73e8a5a..d7f04be 100644 --- a/fbproto/op_batch_completion.go +++ b/fbproto/op_batch_completion.go @@ -1,5 +1,7 @@ package fbproto +import "fmt" + // https://firebirdsql.org/file/documentation/html/en/firebirddocs/wireprotocol/firebird-wire-protocol.html#wireprotocol-batches-execute type BatchDetailedError struct { @@ -19,6 +21,12 @@ type OpBatchCompletion struct { func (op *OpBatchCompletion) Read(x *XDRReader) error { var err error + // Reuse storage, but grow slices only after successfully reading an element. + op.UpdateCounts = op.UpdateCounts[:0] + clear(op.DetailedErrors) + op.DetailedErrors = op.DetailedErrors[:0] + op.SimplifiedErrors = op.SimplifiedErrors[:0] + // p_batch_statement if op.StatementHandle, err = x.ReadInt32(); err != nil { return err @@ -46,24 +54,22 @@ func (op *OpBatchCompletion) Read(x *XDRReader) error { if err != nil { return err } - - if cap(op.UpdateCounts) < updateCountCount { - op.UpdateCounts = make([]int, updateCountCount) - } else { - op.UpdateCounts = op.UpdateCounts[:updateCountCount] + if op.TotalRecords < 0 || updateCountCount < 0 || detailedErrorCount < 0 || simplifiedErrorCount < 0 { + return fmt.Errorf("invalid batch completion counts: records=%d updates=%d vectors=%d errors=%d", + op.TotalRecords, updateCountCount, detailedErrorCount, simplifiedErrorCount) + } + if updateCountCount > op.TotalRecords || detailedErrorCount > op.TotalRecords || + simplifiedErrorCount > op.TotalRecords-detailedErrorCount { + return fmt.Errorf("batch completion counts exceed record count %d", op.TotalRecords) } // Update counters for i := 0; i < updateCountCount; i++ { - if op.UpdateCounts[i], err = x.ReadInt32(); err != nil { + count, err := x.ReadInt32() + if err != nil { return err } - } - - if cap(op.DetailedErrors) < detailedErrorCount { - op.DetailedErrors = make([]BatchDetailedError, detailedErrorCount) - } else { - op.DetailedErrors = op.DetailedErrors[:detailedErrorCount] + op.UpdateCounts = append(op.UpdateCounts, count) } // Detailed info @@ -72,24 +78,26 @@ func (op *OpBatchCompletion) Read(x *XDRReader) error { if err != nil { return err } + if row < 0 || row >= op.TotalRecords { + return fmt.Errorf("invalid batch error row %d for %d records", row, op.TotalRecords) + } sv, err := readStatusVector(x, op.Encoding) if err != nil { return err } - op.DetailedErrors[i] = BatchDetailedError{Row: row, Error: sv.toErrorResponse()} - } - - if cap(op.SimplifiedErrors) < simplifiedErrorCount { - op.SimplifiedErrors = make([]int, simplifiedErrorCount) - } else { - op.SimplifiedErrors = op.SimplifiedErrors[:simplifiedErrorCount] + op.DetailedErrors = append(op.DetailedErrors, BatchDetailedError{Row: row, Error: sv.toErrorResponse()}) } // Simplified error blocks for i := 0; i < simplifiedErrorCount; i++ { - if op.SimplifiedErrors[i], err = x.ReadInt32(); err != nil { + row, err := x.ReadInt32() + if err != nil { return err } + if row < 0 || row >= op.TotalRecords { + return fmt.Errorf("invalid batch error row %d for %d records", row, op.TotalRecords) + } + op.SimplifiedErrors = append(op.SimplifiedErrors, row) } return nil diff --git a/fbproto/op_batch_release_test.go b/fbproto/op_batch_release_test.go index 853baf5..98e5014 100644 --- a/fbproto/op_batch_release_test.go +++ b/fbproto/op_batch_release_test.go @@ -2,6 +2,7 @@ package fbproto import ( "bytes" + "io" "testing" "github.com/stretchr/testify/require" @@ -44,3 +45,127 @@ func TestOpBatchSyncWrite(t *testing.T) { 0x00, 0x00, 0x00, byte(Op_batch_sync), }, buf.Bytes()) } + +func TestOpInfoBatchWrite(t *testing.T) { + var buf bytes.Buffer + err := (&OpInfoBatch{ + StatementHandle: 42, + Items: []byte{BatchInfoBufferBytes, BatchInfoDataBytes, BatchInfoBlobsBytes, BatchInfoBlobAlign, BatchInfoBlobHeader}, + BufferLength: 64, + }).Write(NewXDRWriter(&buf)) + require.NoError(t, err) + require.Equal(t, []byte{ + 0, 0, 0, byte(Op_info_batch), + 0, 0, 0, 42, + 0, 0, 0, 0, + 0, 0, 0, 5, 10, 11, 12, 13, 14, 0, 0, 0, // items and XDR padding + 0, 0, 0, 64, + }, buf.Bytes()) +} + +func TestOpBatchCompletionRead(t *testing.T) { + for _, tc := range []struct { + name string + values []int + valid bool + }{ + {"empty", []int{42, 0, 0, 0, 0}, true}, + {"counts and errors", []int{42, 3, 3, 1, 1, 1, -1, -1, 1, Isc_arg_gds, Isc_cancelled, Isc_arg_end, 2}, true}, + {"negative records", []int{42, -1, 0, 0, 0}, false}, + {"negative updates", []int{42, 1, -1, 0, 0}, false}, + {"negative vectors", []int{42, 1, 0, -1, 0}, false}, + {"negative errors", []int{42, 1, 0, 0, -1}, false}, + {"too many updates", []int{42, 1, 2, 0, 0}, false}, + {"too many errors", []int{42, 1, 0, 1, 1}, false}, + {"invalid detailed row", []int{42, 1, 0, 1, 0, -1}, false}, + {"invalid simplified row", []int{42, 1, 0, 0, 1, 1}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + var data bytes.Buffer + w := NewXDRWriter(&data) + for _, n := range tc.values { + require.NoError(t, w.WriteInt32(n)) + } + op := &OpBatchCompletion{} + if !tc.valid { + require.Error(t, op.Read(NewXDRReader(&data))) + return + } + payload := append([]byte(nil), data.Bytes()...) + require.NoError(t, w.WriteInt32(77)) + r := NewXDRReader(&data) + require.NoError(t, op.Read(r)) + require.Equal(t, 42, op.StatementHandle) + next, err := r.ReadInt32() + require.NoError(t, err) + require.Equal(t, 77, next) + if op.TotalRecords > 0 { + require.Equal(t, []int{1, -1, -1}, op.UpdateCounts) + require.Equal(t, 1, op.DetailedErrors[0].Row) + require.Equal(t, []int{2}, op.SimplifiedErrors) + } + previous := *op + previous.DetailedErrors = append([]BatchDetailedError(nil), op.DetailedErrors...) + require.NoError(t, op.Read(NewXDRReader(bytes.NewReader(payload)))) + require.Equal(t, previous, *op) + require.Equal(t, cap(previous.UpdateCounts), cap(op.UpdateCounts)) + require.Equal(t, cap(previous.DetailedErrors), cap(op.DetailedErrors)) + require.Equal(t, cap(previous.SimplifiedErrors), cap(op.SimplifiedErrors)) + require.ErrorIs(t, op.Read(NewXDRReader(bytes.NewReader(payload[:len(payload)-1]))), io.ErrUnexpectedEOF) + var empty bytes.Buffer + for _, n := range []int{42, 0, 0, 0, 0} { + require.NoError(t, NewXDRWriter(&empty).WriteInt32(n)) + } + require.NoError(t, op.Read(NewXDRReader(&empty))) + require.Empty(t, op.UpdateCounts) + require.Empty(t, op.DetailedErrors) + require.Empty(t, op.SimplifiedErrors) + require.Equal(t, cap(previous.DetailedErrors), cap(op.DetailedErrors)) + for _, detail := range op.DetailedErrors[:cap(op.DetailedErrors)] { + require.Equal(t, BatchDetailedError{}, detail) + } + }) + } +} + +func TestOpBatchCompletionReadTruncated(t *testing.T) { + const count = 1 << 20 + for _, tc := range []struct { + name string + counts []int + element []int + }{ + {"updates", []int{count, 0, 0}, []int{1}}, + {"detailed errors", []int{0, count, 0}, []int{0, Isc_arg_gds, Isc_cancelled, Isc_arg_end}}, + {"simplified errors", []int{0, 0, count}, []int{0}}, + } { + t.Run(tc.name, func(t *testing.T) { + var data bytes.Buffer + w := NewXDRWriter(&data) + for _, n := range append([]int{42, count}, tc.counts...) { + require.NoError(t, w.WriteInt32(n)) + } + for _, n := range tc.element { + require.NoError(t, w.WriteInt32(n)) + } + for _, size := range []int{20, data.Len() - 1, data.Len()} { + var op OpBatchCompletion + err := op.Read(NewXDRReader(bytes.NewReader(data.Bytes()[:size]))) + if size == data.Len()-1 { + require.ErrorIs(t, err, io.ErrUnexpectedEOF) + } else { + require.ErrorIs(t, err, io.EOF) + } + length := len(op.UpdateCounts) + len(op.DetailedErrors) + len(op.SimplifiedErrors) + capacity := cap(op.UpdateCounts) + cap(op.DetailedErrors) + cap(op.SimplifiedErrors) + if size == data.Len() { + require.Equal(t, 1, length) + require.Less(t, capacity, count) + } else { + require.Zero(t, length) + require.Zero(t, capacity) + } + } + }) + } +} diff --git a/fbproto/op_info_batch.go b/fbproto/op_info_batch.go new file mode 100644 index 0000000..ac68dfe --- /dev/null +++ b/fbproto/op_info_batch.go @@ -0,0 +1,24 @@ +package fbproto + +// OpInfoBatch requests information about the batch owned by a statement. +type OpInfoBatch struct { + StatementHandle int + Items []byte + BufferLength int +} + +func (op *OpInfoBatch) Write(x *XDRWriter) error { + if err := x.WriteInt32(Op_info_batch); err != nil { + return err + } + if err := x.WriteInt32(op.StatementHandle); err != nil { + return err + } + if err := x.WriteInt32(0); err != nil { // p_info_incarnation + return err + } + if err := x.WriteBuffer(op.Items); err != nil { + return err + } + return x.WriteInt32(op.BufferLength) +} diff --git a/fbxpool/conn.go b/fbxpool/conn.go index cb1af19..5eb28db 100644 --- a/fbxpool/conn.go +++ b/fbxpool/conn.go @@ -96,22 +96,6 @@ func (c *Conn) PrepareBatchWithOptions(ctx context.Context, sql string, opts fbx return c.Conn().PrepareBatchWithOptions(ctx, sql, opts) } -// Flush sends queued rows for a named batch without executing it. -func (c *Conn) Flush(ctx context.Context, name string) error { - return c.Conn().Flush(ctx, name) -} - -// ExecBatch flushes queued rows for a named batch, executes it, and returns the result. -func (c *Conn) ExecBatch(ctx context.Context, name string) (fbx.BatchResult, error) { - return c.Conn().ExecBatch(ctx, name) -} - -// CancelBatch discards every row queued for a named batch — both the local -// buffer and any rows already flushed to the server. -func (c *Conn) CancelBatch(ctx context.Context, name string) error { - return c.Conn().CancelBatch(ctx, name) -} - func (c *Conn) Query(ctx context.Context, sql string, args ...any) (fbx.Rows, error) { return c.Conn().Query(ctx, sql, args...) } diff --git a/fbxpool/tx.go b/fbxpool/tx.go index ed59432..7a93f5c 100644 --- a/fbxpool/tx.go +++ b/fbxpool/tx.go @@ -64,22 +64,6 @@ func (tx *Tx) PrepareBatchWithOptions(ctx context.Context, sql string, opts fbx. return tx.t.PrepareBatchWithOptions(ctx, sql, opts) } -// Flush sends queued rows for a named batch without executing it. -func (tx *Tx) Flush(ctx context.Context, name string) error { - return tx.t.Flush(ctx, name) -} - -// ExecBatch flushes queued rows for a named batch, executes it, and returns the result. -func (tx *Tx) ExecBatch(ctx context.Context, name string) (fbx.BatchResult, error) { - return tx.t.ExecBatch(ctx, name) -} - -// CancelBatch discards every row queued for a named batch — both the local -// buffer and any rows already flushed to the server. -func (tx *Tx) CancelBatch(ctx context.Context, name string) error { - return tx.t.CancelBatch(ctx, name) -} - func (tx *Tx) Exec(ctx context.Context, sql string, arguments ...any) (fbconn.QueryStatus, error) { return tx.t.Exec(ctx, sql, arguments...) } diff --git a/search_path_test.go b/search_path_test.go index 0e0de73..59b0f10 100644 --- a/search_path_test.go +++ b/search_path_test.go @@ -751,14 +751,13 @@ func TestPreparedBatchSearchPath(t *testing.T) { assert.Equal(t, 3, countIn(schema1)) assert.Equal(t, 2, countIn(schema2)) - _, err = conn.Prepare(ctx, "sp_batch", fmt.Sprintf("batch insert into %s.test(val) values (?)", schema1)) + pb, err = conn.PrepareBatch(ctx, fmt.Sprintf("insert into %s.test(val) values (?)", schema1)) require.NoError(t, err) - _, err = conn.Exec(ctx, "sp_batch", 30) - require.NoError(t, err) - res, err = conn.ExecBatch(ctx, "sp_batch") + require.NoError(t, pb.Add(30)) + res, err = pb.Exec(ctx) require.NoError(t, err) assert.EqualValues(t, 1, res.Affected) - require.NoError(t, conn.Deallocate(ctx, "sp_batch")) + require.NoError(t, pb.Close(ctx)) assert.Equal(t, 4, countIn(schema1)) ensureConnValid(t, conn) diff --git a/stdlib/sql.go b/stdlib/sql.go index c690c62..8e3b56e 100644 --- a/stdlib/sql.go +++ b/stdlib/sql.go @@ -51,6 +51,13 @@ // // db.QueryRowContext(ctx, "select * from users where id=?", userID) // +// Pass one [fbx.BatchRows] argument to ExecContext to execute a complete batch +// with default options. This also works with sql.Stmt and sql.Tx. All encoded +// rows must fit in the server batch buffer; executions are not split into chunks. +// +// db.ExecContext(ctx, "insert into users(id, name) values (?, ?)", +// fbx.BatchRows{{1, "Alice"}, {2, "Bob"}}) +// // Direct query methods also support named placeholder rewriting through // [fbx.NamedArgs]. [sql.Named] names are not interpreted as Firebird // placeholder names. @@ -716,10 +723,10 @@ func (s *Stmt) Close() error { return err } -// NumInput implements [driver.Stmt] and reports the number of positional -// parameters described by Firebird. +// NumInput leaves argument counting to fbx: Exec accepts either ordinary +// parameters or one BatchRows argument containing multiple parameter sets. func (s *Stmt) NumInput() int { - return len(s.sd.Parameters) + return -1 } // Exec implements [driver.Stmt] using a background context. diff --git a/stdlib/sql_test.go b/stdlib/sql_test.go index 9720ebc..2d0d951 100644 --- a/stdlib/sql_test.go +++ b/stdlib/sql_test.go @@ -393,6 +393,85 @@ func TestStmtExec(t *testing.T) { ensureDBValid(t, db) } +func TestExecBatchRows(t *testing.T) { + testWithAllQueryExecModes(t, func(t *testing.T, db *sql.DB) { + ctx := context.Background() + if getServerMajorVersion(t, db) < 4 { + t.Skip("batch requires Firebird 4 or newer") + } + db.SetMaxOpenConns(1) + _, err := db.ExecContext(ctx, "recreate table stdlib_exec_batch (id integer primary key, note varchar(30))") + require.NoError(t, err) + query := "insert into stdlib_exec_batch (id, note) values (?, ?)" + result, err := db.ExecContext(ctx, query, fbx.BatchRows{{1, "one"}, {2, "two"}}) + require.NoError(t, err) + affected, err := result.RowsAffected() + require.NoError(t, err) + require.EqualValues(t, 2, affected) + + stmt, err := db.PrepareContext(ctx, query) + require.NoError(t, err) + defer closeStmt(t, stmt) + _, err = stmt.ExecContext(ctx, fbx.BatchRows{{3, "three"}}) + require.NoError(t, err) + _, err = stmt.ExecContext(ctx, 4, "ordinary") + require.NoError(t, err) + _, err = stmt.ExecContext(ctx, 5) + require.Error(t, err) + _, err = stmt.ExecContext(ctx, fbx.BatchRows{{5, "rolled back"}, {1, "duplicate"}}) + var batchErr *fbx.BatchExecutionError + require.ErrorAs(t, err, &batchErr) + require.False(t, fbconn.SafeToRetry(err)) + _, err = stmt.ExecContext(ctx, fbx.BatchRows{{5}}) + require.ErrorContains(t, err, "expected 2 arguments") + result, err = stmt.ExecContext(ctx, fbx.BatchRows(nil)) + require.NoError(t, err) + affected, err = result.RowsAffected() + require.NoError(t, err) + require.Zero(t, affected) + + var count int + require.NoError(t, db.QueryRowContext(ctx, "select count(*) from stdlib_exec_batch").Scan(&count)) + require.Equal(t, 4, count) + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + _, err = tx.ExecContext(ctx, query, fbx.BatchRows{{5, "first"}}) + require.NoError(t, err) + txStmt, err := tx.PrepareContext(ctx, query) + require.NoError(t, err) + _, err = txStmt.ExecContext(ctx, fbx.BatchRows{{6, "second"}}) + require.NoError(t, err) + _, err = txStmt.ExecContext(ctx, fbx.BatchRows{{1, "duplicate"}}) + require.ErrorAs(t, err, &batchErr) + require.NoError(t, tx.QueryRowContext(ctx, "select count(*) from stdlib_exec_batch").Scan(&count)) + require.Equal(t, 6, count) + require.NoError(t, txStmt.Close()) + require.NoError(t, tx.Rollback()) + require.NoError(t, db.QueryRowContext(ctx, "select count(*) from stdlib_exec_batch").Scan(&count)) + require.Equal(t, 4, count) + + _, err = stmt.ExecContext(ctx, fbx.BatchRows{{7, "reused"}}) + require.NoError(t, err) + tx, err = db.BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + _, err = tx.ExecContext(ctx, query, fbx.BatchRows{{8, "committed"}}) + require.NoError(t, err) + require.NoError(t, tx.Commit()) + require.NoError(t, db.QueryRowContext(ctx, "select count(*) from stdlib_exec_batch").Scan(&count)) + require.Equal(t, 6, count) + + selectStmt, err := db.PrepareContext(ctx, "select cast(? as integer) from rdb$database") + require.NoError(t, err) + defer closeStmt(t, selectStmt) + require.Error(t, selectStmt.QueryRowContext(ctx).Scan(&count)) + require.Error(t, selectStmt.QueryRowContext(ctx, 1, 2).Scan(&count)) + require.NoError(t, selectStmt.QueryRowContext(ctx, 42).Scan(&count)) + require.Equal(t, 42, count) + }) +} + func TestQueryCloseRowsEarly(t *testing.T) { db := openDB(t) defer closeDB(t, db) diff --git a/tx.go b/tx.go index 03cdea8..2e81626 100644 --- a/tx.go +++ b/tx.go @@ -187,7 +187,6 @@ type Tx interface { // Prepare resolves SQL in this transaction and registers the statement on // the connection. The statement remains usable after Commit or Rollback. - // Named batches (SQL prefixed with "batch ") belong to this transaction. Prepare(ctx context.Context, name, sql string) (*fbconn.StatementDescription, error) // PrepareBatch creates a batch object inside the transaction. // @@ -198,17 +197,6 @@ type Tx interface { // // Executing the batch does not commit or roll back the transaction. PrepareBatchWithOptions(ctx context.Context, sql string, opts BatchOptions) (*PreparedBatch, error) - // Flush sends queued rows for a named batch without executing it. - Flush(ctx context.Context, name string) error - // ExecBatch flushes queued rows for a named batch, executes it, and returns - // the result. - // - // ExecBatch does not commit or roll back the transaction. - ExecBatch(ctx context.Context, name string) (BatchResult, error) - // CancelBatch discards every row queued for a named batch — both the local - // buffer and any rows already flushed to the server. The prepared batch - // statement is kept and can be reused. - CancelBatch(ctx context.Context, name string) error Exec(ctx context.Context, sql string, arguments ...any) (commandTag fbconn.QueryStatus, err error) Query(ctx context.Context, sql string, args ...any) (Rows, error) @@ -250,15 +238,14 @@ func (tx *dbTx) Commit(ctx context.Context) error { return ErrTxClosed } - cleanupErr := tx.conn.closePreparedBatchesForTx(ctx, tx) err := tx.tx.Commit(ctx) if err != nil { tx.closed = tx.tx.Closed || tx.conn.IsClosed() - return operationerror.Join(err, cleanupErr) + return err } tx.closed = true - return operationerror.WithRetry(cleanupErr, false) + return nil } // Rollback rolls back the transaction. Rollback will return ErrTxClosed if the @@ -270,16 +257,15 @@ func (tx *dbTx) Rollback(ctx context.Context) error { return ErrTxClosed } - cleanupErr := tx.conn.closePreparedBatchesForTx(ctx, tx) err := tx.tx.Rollback(ctx) tx.closed = true if err != nil { // A rollback failure leaves the connection in an undefined state. tx.conn.die() - return operationerror.Join(err, cleanupErr) + return err } - return operationerror.WithRetry(cleanupErr, false) + return nil } // Exec delegates to the underlying *Conn @@ -340,34 +326,6 @@ func (tx *dbTx) Conn() *Conn { return tx.conn } -// Flush sends queued rows for a named batch without executing it. -func (tx *dbTx) Flush(ctx context.Context, name string) error { - if tx.closed { - return ErrTxClosed - } - return tx.conn.flushTx(ctx, tx, name) -} - -// ExecBatch flushes queued rows for a named batch, executes it, and returns the result. -// -// ExecBatch does not commit or roll back the transaction. -func (tx *dbTx) ExecBatch(ctx context.Context, name string) (BatchResult, error) { - if tx.closed { - return BatchResult{}, ErrTxClosed - } - return tx.conn.execBatchTx(ctx, tx, name) -} - -// CancelBatch discards every row queued for a named batch — both the local -// buffer and any rows already flushed to the server. The prepared batch -// statement is kept and can be reused. -func (tx *dbTx) CancelBatch(ctx context.Context, name string) error { - if tx.closed { - return ErrTxClosed - } - return tx.conn.cancelBatchTx(ctx, tx, name) -} - // dbSimulatedNestedTx represents a simulated nested transaction implemented by a savepoint. type dbSimulatedNestedTx struct { tx Tx @@ -445,34 +403,6 @@ func (sp *dbSimulatedNestedTx) PrepareBatchWithOptions(ctx context.Context, sql return sp.tx.PrepareBatchWithOptions(ctx, sql, opts) } -// Flush sends queued rows for a named batch without executing it. -func (sp *dbSimulatedNestedTx) Flush(ctx context.Context, name string) error { - if sp.closed { - return ErrTxClosed - } - return sp.tx.Flush(ctx, name) -} - -// ExecBatch flushes queued rows for a named batch, executes it, and returns the result. -// -// ExecBatch does not commit or roll back the transaction. -func (sp *dbSimulatedNestedTx) ExecBatch(ctx context.Context, name string) (BatchResult, error) { - if sp.closed { - return BatchResult{}, ErrTxClosed - } - return sp.tx.ExecBatch(ctx, name) -} - -// CancelBatch discards every row queued for a named batch — both the local -// buffer and any rows already flushed to the server. The prepared batch -// statement is kept and can be reused. -func (sp *dbSimulatedNestedTx) CancelBatch(ctx context.Context, name string) error { - if sp.closed { - return ErrTxClosed - } - return sp.tx.CancelBatch(ctx, name) -} - // Query delegates to the underlying Tx func (sp *dbSimulatedNestedTx) Query(ctx context.Context, sql string, args ...any) (Rows, error) { if sp.closed {