Conversation
There was a problem hiding this comment.
🟢 Approval recommended
Reviewed fixes are covered by deterministic regression tests, with no unresolved blocking issues identified.
Pull request overview
Fixes MySQL TPROC-C generation and TPROC-H schema validation issues with deterministic regression coverage.
Changes:
- Corrects customer selection, order handling, and procedure replacement boundaries.
- Fixes uppercase
SUPPLIERscale-factor checks. - Adds generator and schema regression tests.
File summaries
| File | Description |
|---|---|
tests/mysql-generated-regressions.tcl |
Adds generator and schema regression tests. |
src/mysql/mysqloltp.tcl |
Fixes TPROC-C transaction generation. |
src/mysql/mysqlolap.tcl |
Corrects TPROC-H supplier scale checking. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sm-shaw
left a comment
There was a problem hiding this comment.
This looks valid. The issues identified in the client-side TPROC-C and TPROC-H schema-check paths are present in the current code and are worth fixing.
However, I don't think this PR should be merged in its current MySQL-only form.
The important point is that these code paths are not unique to MySQL. The same client-side TPROC-C implementation and TPROC-H schema-check logic is also present in MariaDB and VillageSQL.
For example, the Payment customer-selection code currently appears in all three implementations:
if { [ expr {$namecnt % 2} ] eq 1 } {
set $namecnt [ expr {$namecnt + 1} ]
}
set cust_id_to_query [ lindex $cust_list [ expr {$namecnt / 2} ] ]The same applies to the client-side Order Status logic and the no-stored-procedure replacement code.
The TPROC-H schema-check issue is also present across the MySQL family. The table dictionary contains:
SUPPLIER
but the scale-factor check compares against:
if { $table eq "supplier" } {and queries:
select count(*) from supplierThe equivalent logic is present in:
src/mysql/mysqlolap.tcl
src/mariadb/mariaolap.tcl
src/villagesql/vsqlolap.tcl
For HammerDB we should avoid fixing the same shared behaviour for one database while leaving the equivalent implementations divergent.
Changes required
Please apply the relevant fixes consistently to:
- MySQL
- MariaDB
- VillageSQL
for both the affected TPROC-C client-side transaction generation and the TPROC-H schema check.
The existing database-specific command names and connection handling should remain unchanged, but the transaction and validation semantics should be consistent across the three implementations.
Please also remove:
tests/mysql-generated-regressions.tcl
HammerDB does not maintain a top-level Tcl regression-test suite. We intentionally keep the main source tree lightweight and validate changes through the existing application, workload, CI and platform testing processes rather than adding standalone regression-test files that then become part of the maintained product.
The mocked tests have been useful for identifying the code paths, but before this is accepted the resulting changes should also be validated against real database implementations.
including:
- generation of the client-side TPROC-C driver with stored procedures disabled
- Payment by surname
- Order Status by surname
- Order Status by customer ID
- Order Status for a customer with no orders
- TPROC-H schema checking with a matching scale factor
- TPROC-H schema checking with a deliberately mismatched scale factor
The underlying issues identified here look valid, but the fix should be applied consistently across the MySQL-family implementations rather than creating a MySQL-only behavioural difference.
Fixes #927.
MySQL TPROC-C without stored procedures can generate an invalid basic driver, select the wrong Payment customer when the surname match count is even, and use the randomly generated customer ID instead of the selected surname match in Order Status. Order Status also unpacks an ID lookup as a nested row and reads unset order fields when no orders exist.
Fix the replacement boundary before
prep_statement, use the lower middle customer, preserve the selected ID and surname, flatten single-row customer lookups, and initialize empty order fields. Also fix TPROC-H schema checking: the expected table name isSUPPLIER, so comparison with lowercasesupplierskips the scale-factor check. Query the same uppercase table name to support case-sensitive servers.This PR contains only MySQL generator fixes and regression tests; it has no OceanBase configuration, runtime modules, or database-category changes.
Validation: ran
tests/mysql-generated-regressions.tclin the original HammerDB 6.0 Linux x86-64 CLI on AWS, first with the original generators and then with the changed generator procedures loaded. Baseline: 12 tests, 6 passed / 6 failed. Fixed: 12 passed / 0 failed. Tests cover basic/timed driver generation with stored procedures enabled/disabled, Payment surname groups of 1–4 customers, Order Status selection and empty orders, and matching/mismatched TPROC-H scale factors with a case-sensitive table fixture. Database results are deterministic mocks; this is not a live MySQL server regression. Windows and GUI execution have not been validated.Run from a HammerDB build containing the changes:
hammerdbcli auto tests/mysql-generated-regressions.tcl.