diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bf6ae5e..c31a93c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ Group lines under `Added` / `Changed` / `Fixed` / `Removed`. Append a PR link ### Fixed +- TPC-B fails fast with a named missing query/section instead of running a silent noop iteration when a custom SQL file omits required statements. ([#145](https://github.com/stroppy-io/stroppy/pull/145)) +- TPC-C treats absent customer, warehouse, item, and stock rows as transaction errors, propagates rollback failures instead of reporting an unknown outcome as success, and fails population-validation checks whose aggregate queries error. ([#145](https://github.com/stroppy-io/stroppy/pull/145)) - `stroppy version` now reports the real build version for Docker images (pushed tag), nightly artifacts (`nightly-`), and release archives (release tag), instead of the generic `0.0.0` fallback. ([#144](https://github.com/stroppy-io/stroppy/pull/144)) - Workload help and shell completion expose typed flags accurately, including explicit booleans and contextual defaults, and SQL override positionals reach registered workload bindings. ([#128](https://github.com/stroppy-io/stroppy/pull/128)) - Typed run parameters and SQL sources keep CLI-over-environment precedence, config env names reject case-only collisions, shared driver pool settings remain active alongside driver-specific settings, and invalid pool fields fail clearly instead of being ignored. ([#128](https://github.com/stroppy-io/stroppy/pull/128)) diff --git a/internal/workloads/tpcb/sql_validation_test.go b/internal/workloads/tpcb/sql_validation_test.go new file mode 100644 index 00000000..79e6ba01 --- /dev/null +++ b/internal/workloads/tpcb/sql_validation_test.go @@ -0,0 +1,108 @@ +package tpcb + +import ( + "strings" + "testing" + + "github.com/stroppy-io/stroppy/pkg/bench" +) + +// tpcbSQL assembles a minimal TPC-B SQL document with both required setup +// sections and the five named transaction queries. missing names one +// (section, query) pair to omit so a test can assert validateSQL names it. +func tpcbSQL(missing struct{ section, query string }) string { + var b strings.Builder + + if missing.section != "drop_schema" { + b.WriteString("--+ drop_schema\n--=\nDROP TABLE t;\n") + } + + if missing.section != "create_schema" { + b.WriteString("--+ create_schema\n--=\nCREATE TABLE t (a INT);\n") + } + + b.WriteString("--+ workload_tx_tpcb\n") + + for _, q := range requiredTxQueries { + if missing.section == q.section && missing.query == q.query { + continue + } + + b.WriteString("--= " + q.query + "\nSELECT 1;\n") + } + + return b.String() +} + +func TestValidateSQLAcceptsCompleteFile(t *testing.T) { + if err := validateSQL(bench.ParseSQL(tpcbSQL(struct{ section, query string }{}))); err != nil { + t.Fatalf("validateSQL(complete) = %v, want nil", err) + } +} + +func TestValidateSQLNamesEveryMissingRequiredQuery(t *testing.T) { + for _, q := range requiredTxQueries { + t.Run(q.query, func(t *testing.T) { + err := validateSQL(bench.ParseSQL(tpcbSQL(q))) + if err == nil { + t.Fatalf("validateSQL missing %s/%s = nil, want error", q.section, q.query) + } + + if !strings.Contains(err.Error(), q.section+"/"+q.query) { + t.Fatalf("validateSQL error %q does not name %s/%s", err, q.section, q.query) + } + }) + } +} + +func TestValidateSQLNamesMissingRequiredSection(t *testing.T) { + for _, name := range requiredSetupSections { + t.Run(name, func(t *testing.T) { + err := validateSQL(bench.ParseSQL(tpcbSQL(struct{ section, query string }{section: name}))) + if err == nil { + t.Fatalf("validateSQL missing %q = nil, want error", name) + } + + if !strings.Contains(err.Error(), name) { + t.Fatalf("validateSQL error %q does not name section %q", err, name) + } + }) + } +} + +// tpcbSQLWithEmptyQuery assembles a TPC-B document whose named query for empty +// is declared with a `--= name` marker but no statement body. +func tpcbSQLWithEmptyQuery(empty struct{ section, query string }) string { + var b strings.Builder + + b.WriteString("--+ drop_schema\n--=\nDROP TABLE t;\n") + b.WriteString("--+ create_schema\n--=\nCREATE TABLE t (a INT);\n") + b.WriteString("--+ workload_tx_tpcb\n") + + for _, q := range requiredTxQueries { + b.WriteString("--= " + q.query + "\n") + + if empty.section == q.section && empty.query == q.query { + continue // declared but empty: the next marker follows immediately + } + + b.WriteString("SELECT 1;\n") + } + + return b.String() +} + +func TestValidateSQLRejectsEmptyQueryBody(t *testing.T) { + for _, q := range requiredTxQueries { + t.Run(q.query, func(t *testing.T) { + err := validateSQL(bench.ParseSQL(tpcbSQLWithEmptyQuery(q))) + if err == nil { + t.Fatalf("validateSQL empty %s/%s = nil, want error", q.section, q.query) + } + + if !strings.Contains(err.Error(), "empty query "+q.section+"/"+q.query) { + t.Fatalf("validateSQL error %q does not name empty %s/%s", err, q.section, q.query) + } + }) + } +} diff --git a/internal/workloads/tpcb/tpcb.go b/internal/workloads/tpcb/tpcb.go index f0f30f80..cd441f5b 100644 --- a/internal/workloads/tpcb/tpcb.go +++ b/internal/workloads/tpcb/tpcb.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "math/rand/v2" + "strings" "sync" "sync/atomic" @@ -19,6 +20,28 @@ import ( var errAccountNotFound = errors.New("tpc-b: account not found") +// requiredTxQueries are the named transaction queries the measured iteration +// depends on. Every custom SQL file must provide them; a missing one would +// otherwise parse as an empty statement and run as a silent noop. +var requiredTxQueries = []struct{ section, query string }{ + {"workload_tx_tpcb", "update_account"}, + {"workload_tx_tpcb", "get_balance"}, + {"workload_tx_tpcb", "update_teller"}, + {"workload_tx_tpcb", "update_branch"}, + {"workload_tx_tpcb", "insert_history"}, +} + +// requiredSetupSections are the schema sections the setup steps execute. They are +// present in every dialect (unlike the pg/mysql-only index/fk/analyze sections, +// which legitimately no-op on picodata/ydb). +var requiredSetupSections = []string{"drop_schema", "create_schema"} + +var ( + errMissingSection = errors.New("tpc-b: missing section") + errMissingQuery = errors.New("tpc-b: missing query") + errEmptyQuery = errors.New("tpc-b: empty query") +) + const ( preset = "tpcb" @@ -71,6 +94,10 @@ func (w *workload) Setup(ctx context.Context, b *bench.Bench) error { w.iso = resolveIsolation(w.driverType, w.iso) w.sql = mustLoadSQL(w.driverType, w.sqlFile) + if err := validateSQL(w.sql); err != nil { + return err + } + runSection := func(name string) error { for _, q := range w.sql.Section(name) { if err := b.Exec(ctx, q, nil); err != nil { @@ -218,6 +245,31 @@ func mustLoadSQL(dt bench.DriverTypeName, override string) *bench.SQL { return s } +// validateSQL asserts the schema sections and named transaction queries the +// workload needs are present before measured execution, so a custom SQL file +// that omits TPC-B statements fails with a named missing query/section instead +// of degrading into successful noop iterations. +func validateSQL(sql *bench.SQL) error { + for _, name := range requiredSetupSections { + if len(sql.Section(name)) == 0 { + return fmt.Errorf("%w %q", errMissingSection, name) + } + } + + for _, q := range requiredTxQueries { + body, ok := sql.Query(q.section, q.query) + if !ok { + return fmt.Errorf("%w %s/%s", errMissingQuery, q.section, q.query) + } + + if strings.TrimSpace(body) == "" { + return fmt.Errorf("%w %s/%s", errEmptyQuery, q.section, q.query) + } + } + + return nil +} + // --- per-VU tx-time generators --- type vuState struct { diff --git a/internal/workloads/tpcc/neworder_test.go b/internal/workloads/tpcc/neworder_test.go new file mode 100644 index 00000000..2f942cf5 --- /dev/null +++ b/internal/workloads/tpcc/neworder_test.go @@ -0,0 +1,302 @@ +package tpcc + +import ( + "context" + "errors" + "strings" + "testing" + + "go.uber.org/zap" + + "github.com/stroppy-io/stroppy/pkg/bench" + "github.com/stroppy-io/stroppy/pkg/common/proto/stroppy" + "github.com/stroppy-io/stroppy/pkg/driver" + "github.com/stroppy-io/stroppy/pkg/driver/stats" +) + +// newOrderTestSQL is a minimal workload_tx_new_order section whose query bodies +// carry unique markers so the fake tx can dispatch each read to a canned row set. +const newOrderTestSQL = ` +--+ workload_tx_new_order +--= get_customer +SELECT CUSTOMER_ROW +--= get_warehouse +SELECT WAREHOUSE_ROW +--= get_district +SELECT DISTRICT_ROW +--= update_district +UPDATE_DISTRICT +--= insert_order +INSERT_ORDER_ROW +--= insert_new_order +INSERT_NEW_ORDER +--= get_items_batch +SELECT ITEM_ROWS {item_ids} +--= get_stocks_batch +SELECT STOCK_ROWS {item_ids} +--= update_stock +UPDATE_STOCK_ROW +--= insert_order_line +INSERT_ORDER_LINE +` + +// fakeRows is a minimal driver.Rows over a canned row set. +type fakeRows struct { + rows [][]any + idx int +} + +func (r *fakeRows) Columns() []string { return nil } +func (r *fakeRows) Next() bool { + if r.idx < len(r.rows) { + r.idx++ + + return true + } + + return false +} +func (r *fakeRows) Values() []any { return r.rows[r.idx-1] } +func (r *fakeRows) ReadAll(_ int) [][]any { return r.rows } +func (r *fakeRows) Err() error { return nil } +func (r *fakeRows) Close() error { return nil } + +// fakeTx implements driver.Tx; RunQuery dispatches on the SQL markers above. +type fakeTx struct { + respond func(sql string) ([][]any, error) + commitErr error + rollbackErr error +} + +func (t *fakeTx) RunQuery(_ context.Context, sql string, _ map[string]any) (*driver.QueryResult, error) { + rows, err := t.respond(sql) + if err != nil { + return nil, err + } + + return &driver.QueryResult{Rows: &fakeRows{rows: rows}}, nil +} + +func (t *fakeTx) Commit(context.Context) error { return t.commitErr } +func (t *fakeTx) Rollback(context.Context) error { return t.rollbackErr } +func (t *fakeTx) Isolation() stroppy.TxIsolationLevel { return stroppy.TxIsolationLevel_READ_COMMITTED } + +// fakeDriver implements driver.Driver and hands out a pre-wired tx. +type fakeDriver struct{ tx driver.Tx } + +func (d *fakeDriver) Insert(context.Context, *driver.InsertRequest) (*stats.Query, error) { + return &stats.Query{}, nil +} + +func (d *fakeDriver) RunQuery(context.Context, string, map[string]any) (*driver.QueryResult, error) { + return &driver.QueryResult{}, nil +} + +func (d *fakeDriver) Begin(context.Context, stroppy.TxIsolationLevel) (driver.Tx, error) { + return d.tx, nil +} + +func (d *fakeDriver) ClassifyError(error) driver.ErrorFacts { return driver.ErrorFacts{} } +func (d *fakeDriver) Teardown(context.Context) error { return nil } + +const ( + fakeDriverType = stroppy.DriverConfig_DriverType(99) + newOrderTestWorkloadName = "tpcc/test-new-order" +) + +var ( + currentDriver = &fakeDriver{} + currentRunner = &newOrderRunner{} +) + +func init() { + driver.RegisterDriver(fakeDriverType, func(context.Context, driver.Options) (driver.Driver, error) { + return currentDriver, nil + }) + + bench.Register(func() bench.Workload { return currentRunner }) +} + +// newOrderRunner is a thin workload that invokes the new-order body once with +// canned line items, capturing the resulting error. +type newOrderRunner struct { + w *workload + lineIID []int64 + lineQty []int64 + lineSupply []int64 + forceRollback bool + err error +} + +func (*newOrderRunner) Name() string { return newOrderTestWorkloadName } +func (*newOrderRunner) Define(*bench.Def) error { return nil } +func (*newOrderRunner) Setup(context.Context, *bench.Bench) error { return nil } +func (*newOrderRunner) Teardown(context.Context, *bench.Bench) error { return nil } + +func (r *newOrderRunner) Iterate(ctx context.Context, b *bench.Bench) error { + tx, err := b.Begin(ctx, bench.BeginOpts{Isolation: bench.IsoReadCommitted, Name: "new_order"}) + if err != nil { + r.err = err + + return err + } + + r.err = r.w.newOrderBody(ctx, tx, + 1, 1, 1, int64(len(r.lineIID)), 1, + r.lineIID, r.lineQty, r.lineSupply, r.forceRollback) + + return r.err +} + +// runNewOrderBody executes newOrderBody once against the given tx row responses +// and returns the resulting error. +func runNewOrderBody( + t *testing.T, + respond func(sql string) ([][]any, error), + lineIID []int64, + forceRollback bool, +) error { + t.Helper() + + w := &workload{sql: bench.ParseSQL(newOrderTestSQL), variant: "tx"} + + lineQty := make([]int64, len(lineIID)) + lineSupply := make([]int64, len(lineIID)) + + for i := range lineIID { + lineQty[i] = 5 + lineSupply[i] = 1 + } + + currentDriver = &fakeDriver{tx: &fakeTx{respond: respond}} + currentRunner = &newOrderRunner{ + w: w, lineIID: lineIID, lineQty: lineQty, lineSupply: lineSupply, + forceRollback: forceRollback, + } + + if err := bench.Run( + context.Background(), + newOrderTestWorkloadName, + map[int]*stroppy.DriverConfig{0: {DriverType: fakeDriverType}}, + nil, + bench.ParamInputs{}, + zap.NewNop(), + &bench.MetricsConfig{}, + ); err != nil { + t.Fatalf("bench.Run failed: %v", err) + } + + return currentRunner.err +} + +// newOrderResponds returns a responder keyed by the SQL markers above. For each +// issued query it returns the rows registered for the first marker contained in +// the SQL (or no rows/error for unregistered reads and Exec statements). +func newOrderResponds(m map[string][][]any) func(sql string) ([][]any, error) { + return func(sql string) ([][]any, error) { + for marker, rows := range m { + if strings.Contains(sql, marker) { + return rows, nil + } + } + + return nil, nil + } +} + +func customerRow() []any { return []any{int64(1)} } +func warehouseRow() []any { return []any{int64(1)} } +func districtRow() []any { return []any{int64(2101)} } +func itemRow(iid int64) []any { return []any{iid, float64(10)} } +func stockRow(iid int64) []any { return []any{iid, int64(50), "", "dist"} } + +func TestNewOrderBodyMissingCustomer(t *testing.T) { + err := runNewOrderBody(t, newOrderResponds(map[string][][]any{ + "WAREHOUSE_ROW": {warehouseRow()}, + "DISTRICT_ROW": {districtRow()}, + }), []int64{1}, false) + if !errors.Is(err, errNewOrderCustomerMissing) { + t.Fatalf("missing customer error = %v, want %v", err, errNewOrderCustomerMissing) + } +} + +func TestNewOrderBodyMissingWarehouse(t *testing.T) { + err := runNewOrderBody(t, newOrderResponds(map[string][][]any{ + "CUSTOMER_ROW": {customerRow()}, + "DISTRICT_ROW": {districtRow()}, + }), []int64{1}, false) + if !errors.Is(err, errNewOrderWarehouseMissing) { + t.Fatalf("missing warehouse error = %v, want %v", err, errNewOrderWarehouseMissing) + } +} + +func TestNewOrderBodyMissingItem(t *testing.T) { + err := runNewOrderBody(t, newOrderResponds(map[string][][]any{ + "CUSTOMER_ROW": {customerRow()}, + "WAREHOUSE_ROW": {warehouseRow()}, + "DISTRICT_ROW": {districtRow()}, + "ITEM_ROWS": {itemRow(10)}, // item 20 absent + "STOCK_ROWS": {stockRow(10)}, + }), []int64{10, 20}, false) + if !errors.Is(err, errItemNotFound) { + t.Fatalf("missing item error = %v, want %v", err, errItemNotFound) + } +} + +func TestNewOrderBodyForcedRollbackReportsMissingRegularItem(t *testing.T) { + err := runNewOrderBody(t, newOrderResponds(map[string][][]any{ + "CUSTOMER_ROW": {customerRow()}, + "WAREHOUSE_ROW": {warehouseRow()}, + "DISTRICT_ROW": {districtRow()}, + }), []int64{10, items + 1}, true) + if !errors.Is(err, errItemNotFound) { + t.Fatalf("forced rollback with missing regular item error = %v, want %v", err, errItemNotFound) + } +} + +func TestNewOrderBodyMissingStock(t *testing.T) { + err := runNewOrderBody(t, newOrderResponds(map[string][][]any{ + "CUSTOMER_ROW": {customerRow()}, + "WAREHOUSE_ROW": {warehouseRow()}, + "DISTRICT_ROW": {districtRow()}, + "ITEM_ROWS": {itemRow(10)}, + // STOCK_ROWS omitted: item 10 has no stock row. + }), []int64{10}, false) + if !errors.Is(err, errNewOrderStockMissing) { + t.Fatalf("missing stock error = %v, want %v", err, errNewOrderStockMissing) + } +} + +func TestFinishNewOrderSentinelSuccess(t *testing.T) { + if err := finishNewOrder(errRollbackSentinel, nil); err != nil { + t.Fatalf("sentinel + nil rollback = %v, want nil", err) + } +} + +func TestFinishNewOrderSentinelRollbackFailure(t *testing.T) { + rbErr := errors.New("rollback failed") + + err := finishNewOrder(errRollbackSentinel, rbErr) + if err == nil { + t.Fatal("sentinel + rollback error = nil, want error (unknown outcome must not be success)") + } + + if !errors.Is(err, rbErr) { + t.Fatalf("error %v does not wrap rollback error %v", err, rbErr) + } +} + +func TestFinishNewOrderPropagatesRollbackError(t *testing.T) { + rbErr := errors.New("rollback failed") + + err := finishNewOrder(errItemNotFound, rbErr) + if !errors.Is(err, errItemNotFound) || !errors.Is(err, rbErr) { + t.Fatalf("error %v should wrap both body %v and rollback %v", err, errItemNotFound, rbErr) + } +} + +func TestFinishNewOrderPlainError(t *testing.T) { + if err := finishNewOrder(errItemNotFound, nil); !errors.Is(err, errItemNotFound) { + t.Fatalf("error = %v, want %v", err, errItemNotFound) + } +} diff --git a/internal/workloads/tpcc/tpcc.go b/internal/workloads/tpcc/tpcc.go index 24d82eef..61a25be7 100644 --- a/internal/workloads/tpcc/tpcc.go +++ b/internal/workloads/tpcc/tpcc.go @@ -24,9 +24,18 @@ import ( var txNames = []string{"new_order", "payment", "order_status", "delivery", "stock_level"} var ( - errProcsDriverUnsupported = errors.New("tpcc/procs only supports postgres and mysql; use tpcc/tx for picodata/ydb") - errDistrictNotFound = errors.New("new_order: district not found") - errItemNotFound = errors.New("tpcc_rollback:item_not_found") + errProcsDriverUnsupported = errors.New("tpcc/procs only supports postgres and mysql; use tpcc/tx for picodata/ydb") + errDistrictNotFound = errors.New("new_order: district not found") + errNewOrderCustomerMissing = errors.New("new_order: customer not found") + errNewOrderWarehouseMissing = errors.New("new_order: warehouse not found") + errItemNotFound = errors.New("new_order: item not found") + errNewOrderStockMissing = errors.New("new_order: stock not found") + + // errRollbackSentinel marks the spec-mandated invalid-item rollback: the + // transaction rolls back on a bogus item id and is reported as success. + // isRollbackSentinel matches this value and the equivalent server-side raise. + errRollbackSentinel = errors.New("tpcc_rollback:item_not_found") + errPaymentNoCustomers = errors.New("payment: no customers match c_last") errPaymentByNameNoRow = errors.New("payment: by-name SELECT returned no row") errPaymentCustomerMissing = errors.New("payment: customer not found") @@ -389,36 +398,60 @@ func (w *workload) newOrder(ctx context.Context, b *bench.Bench, vs *vuState) er if err := w.newOrderBody(ctx, tx, wID, dID, cID, olCnt, allLocal, lineIID, lineQty, lineSupply, forceRollback); err != nil { - _ = tx.Rollback(ctx) - - if isRollbackSentinel(err) { - return nil // spec-mandated rollback counts as success - } - - return err + return finishNewOrder(err, tx.Rollback(ctx)) } return tx.Commit(ctx) }) } -//nolint:gocognit,cyclop,funlen // TPC-C spec transaction; complexity is inherent to the spec. +// finishNewOrder maps a new-order body error and its rollback error to the +// transaction result. The spec-mandated invalid-item rollback (sentinel body +// error) counts as success only when the rollback completed; any rollback error +// is propagated so an unknown transaction outcome is never reported as success. +func finishNewOrder(bodyErr, rollbackErr error) error { + if rollbackErr != nil { + if isRollbackSentinel(bodyErr) { + return fmt.Errorf("new_order: rollback after spec rollback: %w", rollbackErr) + } + + return errors.Join(bodyErr, rollbackErr) + } + + if isRollbackSentinel(bodyErr) { + return nil + } + + return bodyErr +} + +//nolint:gocognit,gocyclo,cyclop,funlen // TPC-C spec transaction; complexity is inherent to the spec. func (w *workload) newOrderBody( ctx context.Context, tx *bench.TxX, wID, dID, cID, olCnt, allLocal int64, lineIID, lineQty, lineSupply []int64, forceRollback bool, ) error { - if _, err := tx.QueryRow(ctx, w.q("workload_tx_new_order", "get_customer"), map[string]any{ + custRow, err := tx.QueryRow(ctx, w.q("workload_tx_new_order", "get_customer"), map[string]any{ "c_id": cID, "d_id": dID, "w_id": wID, - }); err != nil { + }) + if err != nil { return err } - if _, err := tx.QueryRow(ctx, w.q("workload_tx_new_order", "get_warehouse"), map[string]any{"w_id": wID}); err != nil { + if custRow == nil { + return fmt.Errorf("%w: (%d,%d,%d)", errNewOrderCustomerMissing, wID, dID, cID) + } + + whRow, err := tx.QueryRow(ctx, w.q("workload_tx_new_order", "get_warehouse"), map[string]any{"w_id": wID}) + if err != nil { return err } + if whRow == nil { + return fmt.Errorf("%w: %d", errNewOrderWarehouseMissing, wID) + } + distRow, err := tx.QueryRow(ctx, w.q("workload_tx_new_order", "get_district"), map[string]any{ "d_id": dID, "w_id": wID, }) @@ -465,9 +498,15 @@ func (w *workload) newOrderBody( } if forceRollback && !itemMapHas(itemMap, lineIID[olCnt-1]) { + for _, iid := range lineIID[:olCnt-1] { + if !itemMapHas(itemMap, iid) { + return fmt.Errorf("%w: %d", errItemNotFound, iid) + } + } + w.m.rollbackDone.Add(1) - return errItemNotFound + return errRollbackSentinel } // Batch stock read, grouped by supply warehouse. @@ -512,16 +551,14 @@ func (w *workload) newOrderBody( itemRow, ok := itemMap[iid] if !ok { - w.m.rollbackDone.Add(1) - - return errItemNotFound + return fmt.Errorf("%w: %d", errItemNotFound, iid) } iPrice := toFloat64(itemRow[1]) stockRow, ok := stockMap[stockKey{supplyWID, iid}] if !ok { - continue // skip line if stock missing (matches tx.ts) + return fmt.Errorf("%w: (%d,%d)", errNewOrderStockMissing, supplyWID, iid) } sQuantityOld := toInt64(stockRow[1]) diff --git a/internal/workloads/tpcc/validate.go b/internal/workloads/tpcc/validate.go index 35858b1d..202ed479 100644 --- a/internal/workloads/tpcc/validate.go +++ b/internal/workloads/tpcc/validate.go @@ -31,13 +31,15 @@ func validatePopulation(ctx context.Context, b *bench.Bench, warehouses, warehou return err } - cc1WSum, _ := qfloat(ctx, b, "SELECT SUM(w_ytd) FROM warehouse WHERE w_id "+wRange) - cc1DSum, _ := qfloat(ctx, b, "SELECT SUM(d_ytd) FROM district "+wWhere("d_w_id")) - cc4OSum, _ := qint(ctx, b, "SELECT SUM(o_ol_cnt) FROM orders "+wWhere("o_w_id")) - cc4OlCnt, _ := qint(ctx, b, "SELECT COUNT(*) FROM order_line "+wWhere("ol_w_id")) + cc1WSum, cc1WErr := qfloat(ctx, b, "SELECT SUM(w_ytd) FROM warehouse WHERE w_id "+wRange) + cc1DSum, cc1DErr := qfloat(ctx, b, "SELECT SUM(d_ytd) FROM district "+wWhere("d_w_id")) + cc4OSum, cc4OErr := qint(ctx, b, "SELECT SUM(o_ol_cnt) FROM orders "+wWhere("o_w_id")) + cc4OlCnt, cc4OlErr := qint(ctx, b, "SELECT COUNT(*) FROM order_line "+wWhere("ol_w_id")) checkCardinalities(ctx, b, check, wWhere, wRange, warehouses) - checkConsistency(check, distNext, ordMax, noStats, cc1WSum, cc1DSum, cc4OSum, cc4OlCnt) + checkConsistency(check, distNext, ordMax, noStats, + cc1WSum, cc1DSum, cc1WErr, cc1DErr, + cc4OSum, cc4OlCnt, cc4OErr, cc4OlErr) checkDistribution(ctx, b, check, wWhere, wRange) if len(failures) > 0 { @@ -119,13 +121,20 @@ func checkCardinalities( } // checkConsistency runs the CC1–CC4 logical-consistency checks against the -// prefetched district/order/new_order aggregates. +// prefetched district/order/new_order aggregates. A CC1/CC4 aggregate query error +// fails its check outright, before any comparison: two zero fallback values could +// otherwise compare equal and spuriously pass. func checkConsistency( check func(string, bool), distNext map[string]int64, ordMax map[string]int64, noStats map[string]noStat, - cc1WSum, cc1DSum float64, cc4OSum, cc4OlCnt int64, + cc1WSum, cc1DSum float64, cc1WErr, cc1DErr error, + cc4OSum, cc4OlCnt int64, cc4OErr, cc4OlErr error, ) { - check("CC1 sum(W_YTD) = sum(D_YTD)", absf(cc1WSum-cc1DSum) < 0.01) + if cc1WErr != nil || cc1DErr != nil { + check("CC1 sum(W_YTD) = sum(D_YTD)", false) + } else { + check("CC1 sum(W_YTD) = sum(D_YTD)", absf(cc1WSum-cc1DSum) < 0.01) + } for k, dNext := range distNext { check("CC2a D_NEXT_O_ID-1 = max(O_ID) ["+k+"]", ordMax[k] == dNext-1) @@ -134,7 +143,11 @@ func checkConsistency( check("CC3 new_order contiguous ["+k+"]", st.max-st.min+1 == st.cnt) } - check("CC4 sum(O_OL_CNT) = count(order_line)", cc4OSum == cc4OlCnt) + if cc4OErr != nil || cc4OlErr != nil { + check("CC4 sum(O_OL_CNT) = count(order_line)", false) + } else { + check("CC4 sum(O_OL_CNT) = count(order_line)", cc4OSum == cc4OlCnt) + } } // checkDistribution runs the §1.3.1 data-distribution and constant-column checks. diff --git a/internal/workloads/tpcc/validate_test.go b/internal/workloads/tpcc/validate_test.go new file mode 100644 index 00000000..4756e772 --- /dev/null +++ b/internal/workloads/tpcc/validate_test.go @@ -0,0 +1,105 @@ +package tpcc + +import ( + "errors" + "testing" +) + +const ( + cc1Name = "CC1 sum(W_YTD) = sum(D_YTD)" + cc4Name = "CC4 sum(O_OL_CNT) = count(order_line)" +) + +// runConsistency invokes checkConsistency with empty district/order aggregates +// (so only CC1/CC4 fire) and returns the names of passed and failed checks. +func runConsistency( + t *testing.T, + cc1WSum, cc1DSum float64, cc1WErr, cc1DErr error, + cc4OSum, cc4OlCnt int64, cc4OErr, cc4OlErr error, +) (passed, failed []string) { + t.Helper() + + check := func(name string, ok bool) { + if ok { + passed = append(passed, name) + } else { + failed = append(failed, name) + } + } + + checkConsistency(check, + map[string]int64{}, map[string]int64{}, map[string]noStat{}, + cc1WSum, cc1DSum, cc1WErr, cc1DErr, + cc4OSum, cc4OlCnt, cc4OErr, cc4OlErr, + ) + + return passed, failed +} + +func has(list []string, want string) bool { + for _, got := range list { + if got == want { + return true + } + } + + return false +} + +func TestCheckConsistencyCC1QueryErrorFailsCheck(t *testing.T) { + cases := []struct { + name string + werr, derr error + }{ + {"warehouse sum error", errors.New("boom"), nil}, + {"district sum error", nil, errors.New("boom")}, + {"both sums error", errors.New("boom"), errors.New("boom")}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Equal (zero) sums that would spuriously pass without the error guard. + _, failed := runConsistency(t, 0, 0, tc.werr, tc.derr, 0, 0, nil, nil) + if !has(failed, cc1Name) { + t.Fatalf("CC1 did not fail on query error; failed=%v", failed) + } + }) + } +} + +func TestCheckConsistencyCC4QueryErrorFailsCheck(t *testing.T) { + cases := []struct { + name string + oerr, olErr error + }{ + {"orders sum error", errors.New("boom"), nil}, + {"order_line count error", nil, errors.New("boom")}, + {"both error", errors.New("boom"), errors.New("boom")}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Equal (zero) counts that would spuriously pass without the error guard. + _, failed := runConsistency(t, 0, 0, nil, nil, 0, 0, tc.oerr, tc.olErr) + if !has(failed, cc4Name) { + t.Fatalf("CC4 did not fail on query error; failed=%v", failed) + } + }) + } +} + +func TestCheckConsistencyAggregatesStillComparedWhenNoError(t *testing.T) { + t.Run("equal sums pass", func(t *testing.T) { + passed, failed := runConsistency(t, 100, 100, nil, nil, 42, 42, nil, nil) + if !has(passed, cc1Name) || !has(passed, cc4Name) { + t.Fatalf("equal sums should pass CC1/CC4; passed=%v failed=%v", passed, failed) + } + }) + + t.Run("unequal sums fail", func(t *testing.T) { + _, failed := runConsistency(t, 100, 200, nil, nil, 42, 43, nil, nil) + if !has(failed, cc1Name) || !has(failed, cc4Name) { + t.Fatalf("unequal sums should fail CC1/CC4; failed=%v", failed) + } + }) +}