checker: support HeatWave grants in precheck

[GMHDBJD]

What problem does this PR solve?

Issue Number: ref #12751

This PR fixes DM precheck compatibility with Oracle HeatWave/MySQL 8 style SHOW GRANTS output.

Problems addressed:

• HeatWave may return role grants with WITH ADMIN OPTION, which TiDB parser cannot parse as GrantRoleStmt yet.
• HeatWave may return partial revoke statements in SHOW GRANTS; overlapping revokes must not be reported as sufficient privileges.
• HeatWave grants FLUSH_TABLES as a dynamic privilege. It should satisfy the FTWRL dump privilege requirement currently modeled as RELOAD in DM precheck.

What is changed and how it works?

• Add a DM checker-local showGrants helper to tolerate role grants with WITH ADMIN OPTION while still discovering active roles via SHOW GRANTS ... USING ....
• Make role discovery best-effort: unsupported non-role grant syntax, such as MariaDB-specific grant rows, no longer fails before privilege verification can run with the correct parser mode.
• Skip role grants with WITH ADMIN OPTION during direct privilege verification, because role privileges are collected through SHOW GRANTS ... USING ....
• Model overlapping REVOKE statements against the required privilege set, so a global grant plus a partial revoke on a checked schema/table fails conservatively.
• Treat global extended privilege FLUSH_TABLES as satisfying the dump privilege checker’s FTWRL requirement, without treating sibling FLUSH_* dynamic privileges as equivalent.
• Add unit coverage for HeatWave-style grants, role grants with WITH ADMIN OPTION, partial revokes, MariaDB-specific grant rows in role discovery, non-ASCII role grant suffix trimming, and FLUSH_TABLES dynamic privilege handling.

Check List

Tests:

• Unit test
• Manual test with real HeatWave source

Manual validation:

• go test ./dm/pkg/checker ./dm/checker
• Built dm-master, dm-worker, and dmctl from this branch.
• Ran DM against Oracle HeatWave on AWS MySQL 9.7 with precheck enabled and without ignore-checking-items.
• check-task passed with only the MySQL version warning.
• start-task succeeded.
• Full load plus incremental insert/update/delete synced correctly to TiDB.

Related issue: #12751

Release note

Fix DM precheck compatibility with Oracle HeatWave `SHOW GRANTS` output and dynamic `FLUSH_TABLES` privileges.

[gemini-code-assist]

Code Review

This pull request replaces dbutil.ShowGrants with a local showGrants implementation to better handle MySQL 8.0 and HeatWave role grants, specifically ignoring or trimming the WITH ADMIN OPTION suffix. It also adds support for ignoring REVOKE statements and mapping the FLUSH_TABLES privilege to RELOAD. The reviewer feedback highlights two important issues: first, trimAdminOption can potentially panic or slice incorrectly when handling non-ASCII characters due to index mismatches after calling strings.ToUpper; second, failing the entire showGrants execution when a single grant statement fails to parse during role discovery is too fragile and should instead log a warning and continue.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Using strings.ToUpper to find the index of a substring and then slicing the original string grant using that index can lead to panics or incorrect slicing if the string contains non-ASCII characters (e.g., in usernames or role names). This is because strings.ToUpper can change the byte length of certain Unicode characters (such as Turkish 'i' or other special characters), causing the byte indices to shift.\n\nSince WITH ADMIN OPTION is expected to be a suffix, we can safely perform a case-insensitive suffix check and slice using the known ASCII suffix length.

func trimAdminOption(grant string) string {\n\ttrimmed := strings.TrimSpace(grant)\n\tconst suffix = \" WITH ADMIN OPTION\"\n\tif len(trimmed) >= len(suffix) && strings.EqualFold(trimmed[len(trimmed)-len(suffix):], suffix) {\n\t\treturn strings.TrimSpace(trimmed[:len(trimmed)-len(suffix)])\n\t}\n\treturn grant\n}

[gemini-code-assist]

Failing the entire showGrants function (and thus the entire precheck) if a single grant statement fails to parse during role discovery is too restrictive and fragile. If a user has a custom or unsupported grant syntax that is ignored during privilege verification (via shouldIgnoreGrant), it will still cause a hard failure here.\n\nSince this loop is only used for best-effort role discovery, we should log a warning and continue instead of returning an error.

\t\tnode, err := p.ParseOneStmt(grantForParse, \"\", \"\")\n\t\tif err != nil {\n\t\t\tlog.L().Warn(\"failed to parse grant statement during role discovery\", zap.String(\"grant\", grant), zap.Error(err))\n\t\t\tcontinue\n\t\t}

[D3Hunter]

Summary

• Total findings: 4
• Inline comments: 4
• Summary-only findings (no inline anchor): 0

Findings (highest risk first)

⚠️ [Major] (2)

1. Partial revokes can be reported as sufficient privileges (dm/pkg/checker/privilege.go:320)
2. Version-specific MariaDB grants can fail before MariaDB parsing is enabled (dm/pkg/checker/privilege.go:501)

🟡 [Minor] (1)

1. Grant predicate name hides the role-admin case (dm/pkg/checker/privilege.go:505 and dm/pkg/checker/privilege.go:534)

ℹ️ [Info] (1)

1. FLUSH_TABLES-to-RELOAD mapping needs rationale (dm/pkg/checker/privilege.go:336)

[D3Hunter]

[P2] This partial-revoke path treats schema names as patterns. CompilePattern makes _ and % wildcards, but MySQL partial revokes are schema-level restrictions and, when partial_revokes is enabled, unescaped _ and % in schema names are interpreted literally.

Readable example:

GRANT SELECT ON *.* TO u1;
REVOKE SELECT ON `db_%`.* FROM u1;

The REVOKE row restricts only the literal schema named db_%; it should not remove SELECT for tables in schemas such as:

db_01.t1
db_foo.t1

With the current matcher, db_% matches those schema names, so a precheck for db_01.t1 can report a missing SELECT privilege even though that partial revoke does not apply. Consider comparing partial-revoke schema names literally, or escaping _ and % before calling CompilePattern, and add a regression test for literal _/% schema names.

[D3Hunter]

Suggestion: consider modeling SHOW GRANTS as a privilege snapshot before comparing it with requiredPrivs.

The current implementation starts from lackPrivs, subtracts requirements when it sees a GRANT, then adds requirements back when it sees a partial REVOKE. That is workable for this patch, but the mental model is a little indirect because SHOW GRANTS is not really an ordered mutation log. It is a description of effective grants plus restrictions. A model like “parse grants/revokes into a checker-local privilege abstraction, then ask whether each required privilege is satisfied” would be more direct and easier to review.

One possible shape:

type privilegeSnapshot struct {
    grants       []privilegeFact
    restrictions []privilegeFact // partial REVOKE rows from SHOW GRANTS
}

type privilegeFact struct {
    privs []*ast.PrivElem
    level *ast.GrantLevel
}

type requiredScope struct {
    needGlobal bool
    db         string
    table      string
}

Then VerifyPrivileges could become:

func VerifyPrivileges(
    grants []string,
    requiredPrivs map[mysql.PrivilegeType]priv,
    version string,
) (map[mysql.PrivilegeType]priv, error) {
    if len(grants) == 0 {
        return nil, errors.New("there is no such grant defined for current user on host '%%'")
    }

    snapshot, err := buildPrivilegeSnapshot(grants, version)
    if err != nil {
        return nil, err
    }

    return snapshot.Missing(requiredPrivs), nil
}

And the core evaluation could be requirement-driven:

func (s *privilegeSnapshot) Missing(required map[mysql.PrivilegeType]priv) map[mysql.PrivilegeType]priv {
    missing := cloneRequiredPrivs(required)

    for privName, requirement := range required {
        if requirement.needGlobal {
            if s.Has(privName, requiredScope{needGlobal: true}) {
                delete(missing, privName)
            }
            continue
        }

        for dbName, dbReq := range requirement.dbs {
            if dbReq.wholeDB {
                if s.Has(privName, requiredScope{db: dbName}) {
                    delete(missing[privName].dbs, dbName)
                }
                continue
            }

            for tableName := range dbReq.tables {
                if s.Has(privName, requiredScope{db: dbName, table: tableName}) {
                    delete(missing[privName].dbs[dbName].tables, tableName)
                }
            }
        }
    }

    pruneEmptyPrivs(missing)
    return missing
}

I would make Has predicate-based rather than materializing all effective privileges. For example, GRANT SELECT ON *.* covers every schema/table except the schemas restricted by partial revoke rows, so the checker should keep facts and evaluate coverage lazily:

func (s *privilegeSnapshot) Has(priv mysql.PrivilegeType, target requiredScope) bool {
    for _, grant := range s.grants {
        if !grant.covers(priv, target) {
            continue
        }
        if s.isRestricted(grant, priv, target) {
            continue
        }
        return true
    }
    return false
}

Suggested helper split:

func buildPrivilegeSnapshot(grants []string, version string) (*privilegeSnapshot, error)
func (s *privilegeSnapshot) Missing(required map[mysql.PrivilegeType]priv) map[mysql.PrivilegeType]priv
func (s *privilegeSnapshot) Has(priv mysql.PrivilegeType, target requiredScope) bool
func (f privilegeFact) covers(priv mysql.PrivilegeType, target requiredScope) bool
func (s *privilegeSnapshot) isRestricted(grant privilegeFact, priv mysql.PrivilegeType, target requiredScope) bool
func pruneEmptyPrivs(missing map[mysql.PrivilegeType]priv)

Pros:

• The model becomes order-independent. SHOW GRANTS rows are treated as facts, not as a sequence of mutations to lackPrivs.
• The code reads closer to the problem statement: parse current grants, then check required privileges.
• Names like restoreRevokedPrivs, restoreRequiredPrivAtLevel, and toRestore become unnecessary; the concepts become grants, partialRevokes, covers, isRestricted, and missing.
• It is easier to test parsing separately from privilege coverage, including ALL, SUPER satisfying REPLICATION CLIENT, and FLUSH_TABLES satisfying the current RELOAD/FTWRL requirement.
• It avoids relying on SHOW GRANTS output order. The current subtract/add-back approach only works naturally if broad grants are processed before partial revokes.

Cons / cautions:

• This can grow into a full MySQL ACL engine if we are not careful. I would keep it checker-local and only model the privilege shapes DM currently checks.
• Partial revoke semantics need to be precise. A partial REVOKE SELECT ON db1.* should invalidate a global SELECT ON *.* grant for db1, but we should be careful about how it interacts with explicit narrower grants such as GRANT SELECT ON db1.t1 if those can appear together. That case is worth either verifying against MySQL/HeatWave or documenting as out of scope.
• The refactor is larger than the current patch. If the goal is the smallest compatibility fix, the current approach is acceptable; if we want long-term readability and fewer corner-case surprises, the snapshot model is a better direction.

In short, I think this PR is solving the right compatibility issue, but the privilege evaluation would be easier to maintain if SHOW GRANTS were first normalized into a small privilegeSnapshot abstraction and lackPrivs were computed from that snapshot, instead of being mutated directly while parsing each grant row.

[D3Hunter]

I checked the focused checker tests. They pass, but the new tests still leave important revoke/role-handling branches unexercised. Suggested targeted coverage below.

[D3Hunter]

Could you add table-driven tests for the revoke-restoration path? The current coverage mostly proves one REVOKE SELECT ON db1.* case, but this function restores privileges across global/db/table levels and then merges them back into lackPrivs.

Suggested cases:

// FLUSH_TABLES satisfied RELOAD, then a global revoke should make it missing again.
grants: []string{
    "GRANT SELECT ON *.* TO `dmtest`@`%`",
    "GRANT FLUSH_TABLES ON *.* TO `dmtest`@`%`",
    "REVOKE FLUSH_TABLES ON *.* FROM `dmtest`@`%`",
}
// expect: lack of RELOAD global (*.*) privilege

// Table-level revoke should only restore the revoked table.
grants: []string{
    "GRANT RELOAD, SELECT ON *.* TO `dmtest`@`%`",
    "REVOKE SELECT ON `db1`.`tb1` FROM `dmtest`@`%`",
}
checkTables: []filter.Table{{Schema: "db1", Name: "tb1"}, {Schema: "db1", Name: "tb2"}}
// expect: only `db1`.`tb1` is reported missing

That would make the new clone/restore/merge behavior observable instead of relying only on the broad HeatWave happy path.

[D3Hunter]

Please cover the special mapping branches here. The focused package coverage shows this helper is only partially exercised, and isGlobalOnlyPriv is currently not reached by tests.

Examples worth adding:

// DB-level ALL revoke should not restore global-only requirements.
grants: []string{
    "GRANT ALL PRIVILEGES ON *.* TO `dmtest`@`%`",
    "REVOKE ALL PRIVILEGES ON `db1`.* FROM `dmtest`@`%`",
}
// For replication checks, expect no missing REPLICATION CLIENT / REPLICATION SLAVE from this DB-level revoke.

// SUPER is accepted as satisfying REPLICATION CLIENT, so revoking SUPER globally should restore that requirement.
grants: []string{
    "GRANT REPLICATION SLAVE, SUPER ON *.* TO `dmtest`@`%`",
    "REVOKE SUPER ON *.* FROM `dmtest`@`%`",
}
// expect: lack of REPLICATION CLIENT global (*.*) privilege

// FLUSH_TABLES is special, but sibling dynamic privileges must not be equivalent.
// REVOKE FLUSH_STATUS ON *.* should not restore RELOAD; REVOKE FLUSH_TABLES ON *.* should.

Direction: prefer adding these as end-to-end cases in TestVerifyDumpPrivileges and TestVerifyReplicationPrivileges, so the user-facing error strings are verified rather than only private helpers.

[D3Hunter]

Since this local helper replaces dbutil.ShowGrants for dump, replication, target, and connection-number checkers, could you add sqlmock coverage for the branches that are still untested?

Suggested cases:

// Named user should keep the original helper behavior.
grants, err := showGrants(ctx, db, "dmuser", "")
// expect query: SHOW GRANTS FOR 'dmuser'@'%'

// Multiple roles should be included in the USING query.
// first rows include: GRANT `r1`@`%`,`r2`@`%` TO `dmtest`@`%` WITH ADMIN OPTION
// expect second query: SHOW GRANTS FOR CURRENT_USER USING `r1`@`%`, `r2`@`%`

// If the second SHOW GRANTS ... USING query fails, showGrants should return that error.

These would protect compatibility with the original helper while covering the HeatWave-specific role-discovery changes.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• dm/OWNERS [D3Hunter]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-07-09 12:33:16.074625031 +0000 UTC m=+284982.110720077: ☑️ agreed by D3Hunter.

[joechenrh]

Seem redundant

[joechenrh]

Not sure if we can trim directly

		node, err := p.ParseOneStmt(trimAdminOption(grant), "", "")

[joechenrh]

Seem duplicated with cloneDBPriv