feat(admin): upgrade sql-metadata to 3.0.1 and bound the cluster table functions

[phacops]

Stacked on #8297 — review that first; this PR's diff is against it.

Bumps sql-metadata 2.11.0 → 3.0.1, pulling in sqlglot 30.16.0 (it replaced sqlparse as the backend), now that getsentry/pypi#2457 put both on the internal index.

Why the upgrade and the check ship together

On 2.11.0 the scoped admin tools were protected from table functions by accident. That version reports url, merge, clusterallreplicas and the rest in Parser.tables, so allowed_tables rejected them as unknown table names:

	FROM clusterAllReplicas('c', system.users), scoped to querylog_dist
sql-metadata 2.11.0	rejected — parser reports clusterallreplicas as a table
sql-metadata 3.0.1	allowed — parser reports nothing

3.0.1 stops reporting them, so that incidental protection disappears and the explicit rejection from #8297 becomes what carries it. Upgrading without the check below would open a live path for the querylog/cardinality/outcomes tools to read any table via clusterAllReplicas.

The check

cluster/clusterAllReplicas stay permitted — fanning a read across replicas is normal in these tools, and they are the two the system-queries validator already allows. But the table they fan out over now has to clear allowed_tables. The parser cannot see that argument, so it is extracted directly, covering all three call forms ClickHouse accepts: ('name', db.table), ('name', db, table), ('name', db.table, key). A call whose table argument cannot be read is rejected rather than waved through. Tracing, which passes no allowed_tables, is unaffected.

This is also what Warden flagged on #8289 (4VG-58Z).

Note that cluster/clusterAllReplicas are not SOURCES members and carry no AccessType::REMOTE check — they are registered in TableFunctionRemote.cpp with allow_readonly = true, and the only check is SELECT on the target table. So revoking SOURCES from the admin users does not cover them, and this check is the layer that does, unless the ClickHouse grants are also narrowed per table.

Two things the upgrade surfaced

• 3.0.1 ships py.typed, so the # type: ignore[import-untyped] on that import became unused and strict mypy rejected it. Removed.
• The ARRAY JOIN massaging in validate_ro_query stays. I expected 3.x to make it dead code and checked before deleting: 3.0.1 still puts array in Parser.tables_aliases for a plain ARRAY JOIN, so it is still load-bearing.

The lockfile diff is 12 insertions / 4 deletions — only the two intended packages move. It was regenerated with uv 0.12.3 to match CI; an older uv rewrites markers across ~200 unrelated lines.

Testing

tests/admin passes on 3.0.1. New tests cover the cluster-function argument in all three call forms, both rejected (system.users, other_table) and allowed (my_table, with a trailing sharding key, and repeated in one query).

https://claude.ai/code/session_01JAWCwVv23LnmaGADQSuQgV

---

Generated by Claude Code

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 15cb471. Configure here.}

[phacops]

On the sharding-key finding (Seer 15907458/0 and Bugbot 9237ee6a — the same claim): I don't believe this is a bug, and I haven't changed the code. Seer has marked it resolved in 4e984a3, but that is only the rebase moving the commit; the extraction is identical.

The documented signature is:

cluster(['cluster_name', db.table, sharding_key][, SETTINGS name = value, ...])
cluster(['cluster_name', db, table, sharding_key][, SETTINGS name = value, ...])

A sharding key only appears after a complete table spec. So cluster('c', my_table, mykey) is db=my_table, table=mykey — not table plus sharding key. With bare identifiers the sharding key is the fourth argument: cluster('c', db, table, mykey).

That is what the code produces, verified against this branch:

cluster('c', my_table, mykey)            -> {'my_table.mykey'}   # db.table, per the signature
cluster('c', db, tbl, mykey)             -> {'db.tbl'}           # sharding key ignored
clusterAllReplicas('c', db.tbl, rand())  -> {'db.tbl'}           # sharding key ignored

The two forms are genuinely ambiguous from syntax alone — cluster('c', a, b) looks the same either way — and the docs resolve it as db, table.

Worth being explicit about the failure direction if I have this wrong: the extracted name would not be in allowed_tables, so the query is rejected. The cost of my being wrong is an operator hitting a confusing rejection, never an unauthorized read. Given the ambiguity, that is the direction I want to fail in.

Happy to change it for a counter-example from a real instance.

---

Generated by Claude Code

[phacops]

Seer 15907613 is a re-file of 15907458 after the rebase — same claim, answered above. Still not changing the parse: the documented signature is cluster(cluster_name, db, table, sharding_key), so a sharding key follows a complete table spec and cluster('c', a, b) is db=a, table=b.

One part of its suggestion was fair though — "adding a test case with a bare-identifier sharding key would help validate" — so that is now pinned rather than left to prose (07db84b):

def test_cluster_function_bare_identifiers_are_db_and_table() -> None:
    validate_ro_query("SELECT * FROM cluster('c', db, tbl)", allowed_tables={"db.tbl"})
    # And the sharding key, when present, is not part of the table.
    validate_ro_query("SELECT * FROM cluster('c', db, tbl, mykey)", allowed_tables={"db.tbl"})
    with pytest.raises(InvalidCustomQuery):
        validate_ro_query("SELECT * FROM cluster('c', db, tbl)", allowed_tables={"db"})

The two readings are indistinguishable from syntax alone, so if someone later concludes ClickHouse resolves it the other way, that test is the single place to flip — and its docstring says which reading it encodes and why.

---

Generated by Claude Code