Skip to content

frontend: optimize SHOW ACCOUNTS account-info SQL#24092

Merged
mergify[bot] merged 3 commits intomatrixorigin:3.0-devfrom
LeftHandCold:show-accounts-account-info-sql-3.0-dev
Apr 10, 2026
Merged

frontend: optimize SHOW ACCOUNTS account-info SQL#24092
mergify[bot] merged 3 commits intomatrixorigin:3.0-devfrom
LeftHandCold:show-accounts-account-info-sql-3.0-dev

Conversation

@LeftHandCold
Copy link
Copy Markdown
Contributor

@LeftHandCold LeftHandCold commented Apr 8, 2026

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #24091

What this PR does / why we need it:

Address #24091 by rewriting getSqlForAccountInfo() so SHOW ACCOUNTS computes database and table counts with separate aggregates instead of joining mo_catalog.mo_tables and mo_catalog.mo_database on account_id before COUNT(DISTINCT ...).
This removes the mo_tables x mo_database amplification that dominated the getAccountInfo() CPU flamegraph, pushes exact account_id filters into the aggregate CTEs for single-account requests, and keeps the rest of the SHOW ACCOUNTS behavior unchanged.
As a low-risk follow-up, this PR also hardens reconstruction of the internal LIKE predicate by escaping single quotes and backslashes, and adds combined SQL-generation plus parser round-trip coverage.

Testing

  • DYLD_LIBRARY_PATH=/Users/shenjiangwei/Work/code/matrixone/thirdparties/install/lib:/Users/shenjiangwei/Work/code/matrixone/lib CGO_CFLAGS=-I/Users/shenjiangwei/Work/code/matrixone/thirdparties/install/include -I/Users/shenjiangwei/Work/code/matrixone/cgo CGO_LDFLAGS=-L/Users/shenjiangwei/Work/code/matrixone/thirdparties/install/lib -L/Users/shenjiangwei/Work/code/matrixone/lib GOFLAGS=-mod=mod go test ./pkg/frontend -run 'Test_(getSqlForAccountInfo|updateStorageSize|updateCount|updateStorageUsageCache(_V2)?|checkStorageUsageCache(_V2)?|GetObjectCount(_V2)?)' -count=1

Rewrite getSqlForAccountInfo to aggregate database and table counts separately and push exact account filters into those aggregates, removing the mo_tables x mo_database amplification while keeping the existing SHOW ACCOUNTS result shape intact.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Escape single quotes and backslashes when rebuilding the SHOW ACCOUNTS LIKE predicate, and add combined SQL-generation and parser round-trip coverage for quoted and backslash-containing patterns.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@XuPeng-SH XuPeng-SH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: SHOW ACCOUNTS SQL Optimization

LGTM. Clean, well-designed optimization.

What this does

  1. Eliminates cross-product amplification — splits the old db_tbl_counts CTE (which joined mo_tables × mo_database on account_id, creating N_tables × N_databases rows per account) into two independent CTEs: db_counts and tbl_counts.
  2. Pushes account_id filter into CTEs — for single-account SHOW ACCOUNTS, the filter is applied inside both CTEs, not just in the final WHERE clause.
  3. Hardens LIKE pattern escapingquoteSQLStringLiteral() escapes \\\\\ and ''', fixing a potential SQL injection in the old fmt.Sprintf("...like '%s'",...) pattern.
  4. Significantly improved tests — named subtests, wantContains/wantNotContains assertions, parser round-trip validation.

Verified

  • fmt.Sprintf argument ordering matches all 4 %s placeholders in the template ✓
  • Semantic equivalence: both old and new use INNER JOINs, so accounts with zero databases/tables are excluded in both (practically fine — every account has mo_catalog + system tables) ✓
  • quoteSQLStringLiteral correctly preserves LIKE pattern semantics through the parse→quote→re-parse cycle ✓
  • Code is well-factored: buildAccountInfoClause, buildAccountInfoCountFilters, and quoteSQLStringLiteral are clean, testable functions ✓

@mergify mergify bot added the queued label Apr 10, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Apr 10, 2026

Merge Queue Status

  • Entered queue2026-04-10 06:14 UTC · Rule: release-3.0
  • Checks skipped · PR is already up-to-date
  • Merged2026-04-10 06:14 UTC · at 2ea3d76777d676f4ebd12caf7bd56d2616580f47

This pull request spent 11 seconds in the queue, including 1 second running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Utils CI (3.0) / Coverage
    • check-neutral = Matrixone Utils CI (3.0) / Coverage
    • check-skipped = Matrixone Utils CI (3.0) / Coverage
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI (3.0) / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI (3.0) / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI (3.0) / SCA Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI (3.0) / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI (3.0) / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI (3.0) / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI (3.0) / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI (3.0) / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI (3.0) / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI (3.0) / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI (3.0) / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI (3.0) / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI (3.0) / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI (3.0) / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI (3.0) / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI (3.0) / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI (3.0) / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI (3.0) / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI (3.0) / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI (3.0) / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI (3.0) / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI (3.0) / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI (3.0) / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI (3.0) / Compatibility Test With Target on Linux/x64(LAUNCH)

@mergify mergify bot merged commit ad0bd49 into matrixorigin:3.0-dev Apr 10, 2026
24 of 25 checks passed
@mergify mergify bot removed the queued label Apr 10, 2026
@LeftHandCold LeftHandCold deleted the show-accounts-account-info-sql-3.0-dev branch April 10, 2026 06:14
LeftHandCold added a commit to LeftHandCold/matrixone that referenced this pull request Apr 10, 2026
Address matrixorigin#24091 by rewriting `getSqlForAccountInfo()` so `SHOW ACCOUNTS` computes database and table counts with separate aggregates instead of joining `mo_catalog.mo_tables` and `mo_catalog.mo_database` on `account_id` before `COUNT(DISTINCT ...)`.
This removes the `mo_tables x mo_database` amplification that dominated the `getAccountInfo()` CPU flamegraph, pushes exact `account_id` filters into the aggregate CTEs for single-account requests, and keeps the rest of the `SHOW ACCOUNTS` behavior unchanged.
As a low-risk follow-up, this PR also hardens reconstruction of the internal `LIKE` predicate by escaping single quotes and backslashes, and adds combined SQL-generation plus parser round-trip coverage.

Approved by: @XuPeng-SH
mergify bot pushed a commit that referenced this pull request Apr 10, 2026
Address #24091 by rewriting `getSqlForAccountInfo()` so `SHOW ACCOUNTS` computes database and table counts with separate aggregates instead of joining `mo_catalog.mo_tables` and `mo_catalog.mo_database` on `account_id` before `COUNT(DISTINCT ...)`. This removes the `mo_tables x mo_database` amplification that dominated the `getAccountInfo()` CPU flamegraph, pushes exact `account_id` filters into the aggregate CTEs for single-account requests, and keeps the rest of the `SHOW ACCOUNTS` behavior unchanged. As a low-risk follow-up, this PR also hardens reconstruction of the internal `LIKE` predicate by escaping single quotes and backslashes, and adds combined SQL-generation plus parser round-trip coverage.

Approved by: @XuPeng-SH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants