fix: make tenant SHOW TABLE STATUS rows consistent for system statement_info#23774
fix: make tenant SHOW TABLE STATUS rows consistent for system statement_info#23774mergify[bot] merged 11 commits intomatrixorigin:mainfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Fixes incorrect Rows output in SHOW TABLE STATUS for certain system tables (notably system.statement_info) when executed under non-sys tenant accounts, where queries are rewritten to sys with tenant filtering.
Changes:
- Add a targeted
count(*)fallback forRowsinhandleShowTableStatusfor tables requiringShouldSwitchToSysAccount(non-sys tenants only). - Route background SQL execution in this path through
ExeSqlInBgSesto simplify unit-test stubbing. - Add unit + distributed regression coverage for the tenant
statement_infoRowsinconsistency.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/frontend/mysql_cmd_executor.go | Adds special-table count(*) fallback and switches stats SQL execution to ExeSqlInBgSes. |
| pkg/frontend/mysql_cmd_executor_test.go | Adds unit tests covering fallback behavior, non-trigger cases, and error paths. |
| test/distributed/cases/dml/show/show7.sql | Adds distributed regression scenario for tenant SHOW TABLE STATUS ... LIKE 'statement_info'. |
| test/distributed/cases/dml/show/show7.result | Captures expected output for the new distributed regression case. |
Comments suppressed due to low confidence (1)
pkg/frontend/mysql_cmd_executor.go:621
- When
specialRowsoverrides theRowscolumn,Avg_row_lengthis still computed usingrows[tblName](frommo_table_rows). Ifmo_table_rowsreturns 0 butcount(*)returns >0 (the bug scenario), SHOW TABLE STATUS will reportRows > 0butAvg_row_length = 0, which is internally inconsistent. Consider computing aneffectiveRowsvalue (use the overridden count when present) and use that consistently for both the Rows assignment and the Avg_row_length division guard/denominator.
if cnt, ok := specialRows[tblName]; ok {
ses.data[idx][3] = cnt
} else {
ses.data[idx][3] = rows[tblName]
}
if rows[tblName] > 0 {
ses.data[idx][4] = sizes[tblName] / rows[tblName]
} else {
ses.data[idx][4] = int64(0)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merge Queue Status
This pull request spent 55 minutes 36 seconds in the queue, including 55 minutes 9 seconds running CI. Required conditions to merge
|
What type of PR is this?
Which issue(s) this PR fixes:
issue #10163
What this PR does / why we need it:
Background / Root Cause
This PR fixes issue #10163: under a tenant account, SHOW TABLE STATUS for system.statement_info could show Rows = 0 (or otherwise inconsistent values) even when tenant-visible rows existed.
Root cause:
SHOW TABLE STATUS originally used mo_table_rows(db, tbl) for Rows.
For special system tables (for example system.statement_info), tenant queries are rewritten to run under the sys account with tenant-level filtering.
mo_table_rows does not always reflect that rewritten tenant-visible result set, so Rows could be inaccurate.
Solution
In handleShowTableStatus, add a targeted fallback path:
For non-sys accounts, and only for tables where ShouldSwitchToSysAccount(db, tbl) is true, run select count() on that table.
Use this count() result to override the Rows column in SHOW TABLE STATUS.
Keep existing behavior for all other tables (mo_table_rows/mo_table_size).
Route internal SQL execution through ExeSqlInBgSes (wrapper) to make stubbing in unit tests straightforward.
Test Coverage
Unit Tests (mysql_cmd_executor_test.go)
Added/covered boundary cases:
Fallback success path: mo_table_rows=0, count()>0, Rows is correctly overridden.
Non-special table: fallback is not triggered.
Sys account: fallback is not triggered.
Empty count() result: keep mo_table_rows.
count() execution error path.
count() decode (GetInt64) error path.
BVT (test/distributed/cases/dml/show/show7.sql/.result)
Added issue regression case with stabilization:
Tenant-session warm-up statements + wait (sleep) to avoid async log timing flakiness.
Pre-check count() > 0 on system.statement_info.
Validate SHOW TABLE STATUS ... LIKE 'statement_info' returns positive Rows (regex-based assertion).
Scope / Risk
Behavior change is limited to: non-sys account + special system tables requiring sys-account switch.
Adds one extra count() query only in that narrow path.
All other SHOW TABLE STATUS behavior remains unchanged.