Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical login-command injection and three moderate SQLite persistence findings remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an independent OceanBase category for MySQL-compatible tenants, reusing MySQL workloads with tenant-specific configuration, counters, timeouts, and data generation.
Changes:
- Adds OceanBase backend dispatch, adapter configuration, and migration support.
- Extends workloads with tenant counters, timeouts, and format routing.
- Adds CLI examples, documentation, and regression tests.
File summaries
| File | Reviewed change | Final review comment |
|---|---|---|
tests/oceanbase.tcl |
OceanBase runtime and dispatch tests. | — |
tests/oceanbase-generated.tcl |
Generated-driver regression tests. | — |
src/oceanbase/obotc.tcl |
Transaction counter dispatch. | — |
src/oceanbase/obopt.tcl |
Backend registry and category dispatch. | — |
src/oceanbase/oboltp.tcl |
TPROC-C dispatch entry points. | — |
src/oceanbase/obolap.tcl |
TPROC-H dispatch entry points. | — |
src/oceanbase/obmet.tcl |
Metrics integration boundary. | — |
src/oceanbase/obci.tcl |
CI integration boundary. | — |
src/oceanbase/mysql/adapter.tcl |
Maps tenant configuration to MySQL workloads. | Critical (1 vote): User-controlled login components are inserted unquoted into generated Tcl; quote generated arguments or validate the exact identifier grammar. |
src/mysql/mysqlotc.tcl |
Shared counter and timeout integration. | — |
src/mysql/mysqloltp.tcl |
Session setup and transaction reporting. | — |
src/mysql/mysqlolap.tcl |
TPROC-H parsing and schema compatibility updates. | — |
src/generic/geninitws.tcl |
Web-service configuration initialization. | Moderate (1 vote): SQLite handles remain open, preventing normalized configuration from being persisted and causing defaults to reapply on startup. |
src/generic/geninitcli.tcl |
CLI configuration initialization. | Moderate (1 vote): SQLite handles remain open, preventing normalized configuration from being persisted and causing defaults to reapply on startup. |
src/generic/geninit.tcl |
GUI configuration initialization. | Moderate (1 vote): SQLite handles remain open, preventing normalized configuration from being persisted and causing defaults to reapply on startup. |
src/generic/gengen.tcl |
Backend-aware data-format resolution. | — |
scripts/tcl/oceanbase/mysql/tproch.tcl |
OceanBase TPROC-H workflow. | — |
scripts/tcl/oceanbase/mysql/tprocc.tcl |
OceanBase TPROC-C workflow. | — |
scripts/tcl/oceanbase/mysql/config.tcl |
Shared OceanBase CLI configuration. | — |
modules/oceanbaseconfig-1.0.tm |
OceanBase configuration normalization. | — |
modules/mysqlcommon-1.0.tm |
Shared MySQL/OceanBase runtime helpers. | — |
modules/jobs-1.0.tm |
OceanBase chart colors. | — |
DocBook/oceanbase-mysql.md |
OceanBase tenant documentation. | — |
config/oceanbase.xml |
OceanBase defaults and options. | — |
config/database.xml |
OceanBase category registration. | — |
Review details
Suppressed comments (3)
src/generic/geninit.tcl:104
- When an existing SQLite configuration is loaded,
SQLite2Dictleaves the globalhdbcommand open (src/generic/genxml.tcl:186-214).Dict2SQLitethen tries to createhdbagain, catches the duplicate-command error, and returns without writing, so this normalization is only in memory and the defaults are re-applied on every GUI startup. Please make the shared SQLite read/write path reuse or close the handle before relying on this persistence call.
Dict2SQLite $key $dbconfdict
src/generic/geninitcli.tcl:100
- When an existing SQLite configuration is loaded,
SQLite2Dictleaves the globalhdbcommand open (src/generic/genxml.tcl:186-214).Dict2SQLitethen tries to createhdbagain, catches the duplicate-command error, and returns without writing, so this normalization is only in memory and the defaults are re-applied on every CLI startup. Please make the shared SQLite read/write path reuse or close the handle before relying on this persistence call.
Dict2SQLite $key $dbconfdict
src/generic/geninitws.tcl:100
- When an existing SQLite configuration is loaded,
SQLite2Dictleaves the globalhdbcommand open (src/generic/genxml.tcl:186-214).Dict2SQLitethen tries to createhdbagain, catches the duplicate-command error, and returns without writing, so this normalization is only in memory and the defaults are re-applied on every web-service startup. Please make the shared SQLite read/write path reuse or close the handle before relying on this persistence call.
Dict2SQLite $key $dbconfdict
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| set tenant [dict get $config connection ob_tenant] | ||
| set cluster [dict get $config connection ob_cluster] | ||
| foreach {group key} {tpcc user tpch tpch_user} { | ||
| dict set mapped $group mysql_$key [username [dict get $config $group ob_$key] $tenant $cluster] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on two standalone MySQL fixes: #926 (generated transactions/schema checking) and #929 (loader error propagation). This PR is temporarily based on
codex/mysql-prerequisitesso those fixes are excluded from the OceanBase adaptation diff. Retarget tomasterafter the fixes are incorporated into the fork.Changes
Add an independent OceanBase database category for MySQL-compatible tenants, covering TPROC-C and TPROC-H while retaining the legacy OceanBase option under MySQL. The CLI selects the category with
dbset db ob.Category entry points dispatch generation, counters, options and validation through
ob_compatibility_mode. MySQL-specific configuration mapping, login construction and workload integration live in a separate adapter. Existing configurations migrate tomysql; unsupported modes, includingoracle, fail explicitly. A future Oracle adapter can reuse the Oracle generators without changing the category entry points.Reuse the existing MySQL workloads with isolated configuration and tenant-session query timeouts. Add tenant-local OceanBase transaction counters shared by timed TPROC-C and its chart. Adapt H abbreviated-month parsing and case-insensitive table lookup. Generic transaction/schema-check fixes are reviewed separately in TPC-Council#926, and loader error handling in TPC-Council#929. The OceanBase-specific runtime no longer contains the loader error wrapper. Include CLI examples, migration support and regression tests. Empty optional fields are supplied by an OceanBase-only configuration normalizer; the shared XML parser is unchanged. Offline C/H data generation obtains its format from the selected tenant adapter through a prefix-based callback.
Validation
Tested with the v6.0 runtime on Linux x86-64 against OceanBase Enterprise 4.3.5.6 and OBProxy 4.4.1.0:
OB TPM includes other activity in the same tenant and is not directly equivalent to MySQL statement counters. These runs validate compatibility, not performance.
Offline data-generation regression also passed on AWS: one C warehouse (9 files) and H SF1 (8 files, 1,500,000 orders and 5,998,868 line items), with table row counts and MySQL date-time formatting checked. Fresh and persisted OceanBase defaults and rejection of unsupported Oracle data generation passed.
Remaining validation
Complete GUI startup, Windows execution, TLS, native MySQL server regression, large/multi-node schemas and schema deletion remain unverified. The Linux GUI test was blocked by the repackaged runtime's embedded console initialization. The bundled Linux MySQL client emitted a non-fatal character-set 45 warning during statistics collection; schema and consistency checks passed.
This PR targets
oceanbase/HammerDB:codex/mysql-prerequisitesfor review in our fork. It does not implement Oracle tenant support or claim complete cross-platform validation.