Skip to content

Commit 0ae2385

Browse files
emanzxclaude
andcommitted
fix(tenant): accept tenant name in DROP/ALTER/PURGE TENANT
Mirror the CREATE TENANT <name> (#119 / 86e8ea9) and SHOW TENANT <name> (#126 / 56a29f1) paths by accepting either a numeric id or a tenant name on the remaining tenant DDL forms. Before this change, DROP/ALTER/PURGE TENANT accepted only a numeric id and returned "ERROR: TENANT ID must be a numeric value" on a bare name, which is inconsistent with the CREATE/SHOW surfaces and prevents common substrate-management workflows like `DROP TENANT IF EXISTS my_test_tenant`. Changes: - Add `resolve_tenant_ref(state, token) -> Result<Option<TenantId>>` helper in `pgwire/ddl/tenant/mod.rs`. Numeric tokens go through the legacy fast path (no catalog access). Non-numeric tokens are trimmed of single quotes and resolved via `Catalog::find_tenant_by_name`. Returns `Ok(None)` on name-not-found so callers can decide between `IF EXISTS` no-op and explicit 42704. - Wire the helper into `drop_tenant`, `alter_tenant`, `purge_tenant`. - Update syntax-hint messages from "<id>" to "<id|name>". - Remove now-unused `TenantId` imports in `alter.rs` and `purge.rs`. DROP TENANT IF EXISTS semantics on unknown names parallel the existing unknown-id behavior — both succeed as no-ops; without IF EXISTS, both return 42704. Tests: new `tests/pgwire_auth_tenants/name_resolution.rs` with 9 cases covering numeric regression, bare names, quoted names, unknown names with and without IF EXISTS, empty-name 42601, and ALTER TENANT parallel coverage. Full `pgwire_auth_tenants` suite passes (32/32). Closes #130. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 25040fd commit 0ae2385

6 files changed

Lines changed: 253 additions & 25 deletions

File tree

nodedb/src/control/server/pgwire/ddl/tenant/alter.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
// SPDX-License-Identifier: BUSL-1.1
22

3-
//! `ALTER TENANT <id> SET QUOTA <field> = <value>` handler.
3+
//! `ALTER TENANT <id|name> SET QUOTA <field> = <value>` handler.
44
//!
55
//! Tenant quotas live in the in-memory `TenantStore` and are not part
66
//! of `StoredTenant`. Quota replication is handled separately from
77
//! the tenant identity record.
8+
//!
9+
//! The tenant reference accepts either a numeric id or a tenant name
10+
//! (single-quoted optional), parallel to `CREATE TENANT <name>` and
11+
//! `SHOW TENANT <name|id>`.
812
913
use pgwire::api::results::{Response, Tag};
1014
use pgwire::error::PgWireResult;
1115

1216
use crate::control::security::audit::AuditEvent;
1317
use crate::control::security::identity::AuthenticatedIdentity;
1418
use crate::control::state::SharedState;
15-
use crate::types::TenantId;
1619

1720
use super::super::super::types::sqlstate_error;
1821

@@ -31,19 +34,22 @@ pub fn alter_tenant(
3134
if parts.len() < 7 {
3235
return Err(sqlstate_error(
3336
"42601",
34-
"syntax: ALTER TENANT <id> SET QUOTA <field> = <value>",
37+
"syntax: ALTER TENANT <id|name> SET QUOTA <field> = <value>",
3538
));
3639
}
3740

38-
let tid: u64 = parts[2]
39-
.parse()
40-
.map_err(|_| sqlstate_error("42601", "TENANT ID must be a numeric value"))?;
41-
let tenant_id = TenantId::new(tid);
41+
// Accept either a numeric id or a tenant name (mirrors CREATE/SHOW/DROP).
42+
let tenant_id = super::resolve_tenant_ref(state, parts[2])?.ok_or_else(|| {
43+
sqlstate_error(
44+
"42704",
45+
&format!("tenant '{}' does not exist", parts[2]),
46+
)
47+
})?;
4248

4349
if !parts[3].eq_ignore_ascii_case("SET") || !parts[4].eq_ignore_ascii_case("QUOTA") {
4450
return Err(sqlstate_error(
4551
"42601",
46-
"expected SET QUOTA after tenant ID",
52+
"expected SET QUOTA after tenant id or name",
4753
));
4854
}
4955

nodedb/src/control/server/pgwire/ddl/tenant/drop.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
// SPDX-License-Identifier: BUSL-1.1
22

3-
//! `DROP TENANT [IF EXISTS] <id>` handler. Migrated to
3+
//! `DROP TENANT [IF EXISTS] <id|name>` handler. Migrated to
44
//! `CatalogEntry::DeleteTenant` in phase 1k.6.
5+
//!
6+
//! Accepts either a numeric tenant id or a tenant name (single-quoted
7+
//! optional), parallel to the `CREATE TENANT <name>` and
8+
//! `SHOW TENANT <name|id>` paths.
59
610
use pgwire::api::results::{Response, Tag};
711
use pgwire::error::PgWireResult;
@@ -51,20 +55,34 @@ pub fn drop_tenant(
5155
if parts.len() < 3 {
5256
return Err(sqlstate_error(
5357
"42601",
54-
"syntax: DROP TENANT [IF EXISTS] <id>",
58+
"syntax: DROP TENANT [IF EXISTS] <id|name>",
5559
));
5660
}
5761

58-
let tid: u64 = parts[2]
59-
.parse()
60-
.map_err(|_| sqlstate_error("42601", "TENANT ID must be a numeric value"))?;
61-
let tenant_id = TenantId::new(tid);
62+
// Accept either a numeric id or a tenant name; mirror the existing
63+
// CREATE TENANT name-resolution path.
64+
let tenant_id = match super::resolve_tenant_ref(state, parts[2])? {
65+
Some(tid) => tid,
66+
None => {
67+
// Name token did not resolve to any tenant.
68+
if if_exists {
69+
return Ok(vec![Response::Execution(Tag::new("DROP TENANT"))]);
70+
}
71+
return Err(sqlstate_error(
72+
"42704",
73+
&format!("tenant '{}' does not exist", parts[2]),
74+
));
75+
}
76+
};
77+
let tid = tenant_id.as_u64();
6278

6379
if tid == 0 {
6480
return Err(sqlstate_error("42501", "cannot drop system tenant (0)"));
6581
}
6682

6783
// `IF EXISTS`: dropping a tenant that does not exist is a no-op success.
84+
// (Numeric-id-not-in-catalog path; the name-not-found case was handled
85+
// above.)
6886
if if_exists && !tenant_exists(state, tenant_id)? {
6987
return Ok(vec![Response::Execution(Tag::new("DROP TENANT"))]);
7088
}

nodedb/src/control/server/pgwire/ddl/tenant/mod.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,64 @@ pub use show::{show_tenant_quota, show_tenant_usage};
3131
pub use show_in_database::{
3232
handle_show_tenant_quota_in_database, handle_show_tenant_usage_in_database,
3333
};
34+
35+
use pgwire::error::PgWireResult;
36+
37+
use crate::control::state::SharedState;
38+
use crate::types::TenantId;
39+
40+
use super::super::types::sqlstate_error;
41+
42+
/// Resolve a tenant reference token to a [`TenantId`], accepting either a
43+
/// numeric id or a tenant name.
44+
///
45+
/// The numeric form is the legacy fast path and requires no catalog access;
46+
/// any `u64`-parseable token returns `Ok(Some(TenantId::new(id)))` whether or
47+
/// not that id currently exists (callers that need existence still rely on
48+
/// their own `tenant_exists`-style check).
49+
///
50+
/// A non-numeric token is treated as a tenant name and resolved via
51+
/// [`find_tenant_by_name`]. Single-quoted names are unwrapped, mirroring the
52+
/// AST `TenantSelector` behavior introduced for the CREATE/SHOW paths.
53+
/// `Ok(None)` is returned if the name does not match any tenant, so the
54+
/// caller can decide between `IF EXISTS` no-op success and an explicit
55+
/// `42704` error.
56+
///
57+
/// Errors:
58+
/// - `42601` — empty token (after quote stripping).
59+
/// - `42601` — non-numeric token but catalog is unavailable.
60+
/// - `XX000` — catalog read failure.
61+
///
62+
/// Used by `DROP TENANT`, `ALTER TENANT SET QUOTA`, and `PURGE TENANT` to
63+
/// accept names in addition to numeric ids, parallel to the existing
64+
/// `CREATE TENANT <name>` and `SHOW TENANT <name>` support.
65+
///
66+
/// [`find_tenant_by_name`]:
67+
/// crate::control::security::credential::store::CredentialStore::catalog
68+
pub(super) fn resolve_tenant_ref(
69+
state: &SharedState,
70+
token: &str,
71+
) -> PgWireResult<Option<TenantId>> {
72+
// Numeric id fast path — legacy compatible.
73+
if let Ok(id) = token.parse::<u64>() {
74+
return Ok(Some(TenantId::new(id)));
75+
}
76+
// Name resolution via catalog.
77+
let name = token.trim_matches('\'');
78+
if name.is_empty() {
79+
return Err(sqlstate_error(
80+
"42601",
81+
"TENANT reference must be a numeric id or a tenant name",
82+
));
83+
}
84+
let catalog = state.credentials.catalog().as_ref().ok_or_else(|| {
85+
sqlstate_error(
86+
"42601",
87+
"cannot resolve tenant by name: catalog unavailable; use numeric id",
88+
)
89+
})?;
90+
Ok(catalog
91+
.find_tenant_by_name(name)
92+
.map_err(|e| sqlstate_error("XX000", &format!("catalog read: {e}")))?
93+
.map(|stored| TenantId::new(stored.tenant_id)))
94+
}

nodedb/src/control/server/pgwire/ddl/tenant/purge.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
// SPDX-License-Identifier: BUSL-1.1
22

3-
//! `PURGE TENANT <id> CONFIRM` — Data Plane meta op that deletes
3+
//! `PURGE TENANT <id|name> CONFIRM` — Data Plane meta op that deletes
44
//! ALL tenant data across every engine. Superuser-only, requires
55
//! the literal `CONFIRM` keyword.
6+
//!
7+
//! The tenant reference accepts either a numeric id or a tenant name
8+
//! (single-quoted optional), parallel to `CREATE TENANT <name>` and
9+
//! `SHOW TENANT <name|id>`.
610
711
use pgwire::api::results::{Response, Tag};
812
use pgwire::error::PgWireResult;
913

1014
use crate::control::security::audit::AuditEvent;
1115
use crate::control::security::identity::AuthenticatedIdentity;
1216
use crate::control::state::SharedState;
13-
use crate::types::TenantId;
1417

1518
use super::super::super::types::sqlstate_error;
1619

@@ -27,12 +30,20 @@ pub async fn purge_tenant(
2730
}
2831

2932
if parts.len() < 4 {
30-
return Err(sqlstate_error("42601", "syntax: PURGE TENANT <id> CONFIRM"));
33+
return Err(sqlstate_error(
34+
"42601",
35+
"syntax: PURGE TENANT <id|name> CONFIRM",
36+
));
3137
}
3238

33-
let tid: u64 = parts[2]
34-
.parse()
35-
.map_err(|_| sqlstate_error("42601", "TENANT ID must be a numeric value"))?;
39+
// Accept either a numeric id or a tenant name (mirrors CREATE/SHOW/DROP).
40+
let tenant_id = super::resolve_tenant_ref(state, parts[2])?.ok_or_else(|| {
41+
sqlstate_error(
42+
"42704",
43+
&format!("tenant '{}' does not exist", parts[2]),
44+
)
45+
})?;
46+
let tid = tenant_id.as_u64();
3647

3748
if tid == 0 {
3849
return Err(sqlstate_error("42501", "cannot purge system tenant (0)"));
@@ -45,8 +56,6 @@ pub async fn purge_tenant(
4556
));
4657
}
4758

48-
let tenant_id = TenantId::new(tid);
49-
5059
state.audit_record(
5160
AuditEvent::AdminAction,
5261
Some(tenant_id),

nodedb/tests/pgwire_auth_tenants.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
//! Pgwire DDL coverage for the tenant surface. Split into submodules
44
//! under `pgwire_auth_tenants/`:
55
//!
6-
//! - `lifecycle` — CREATE / DROP TENANT, `IF [NOT] EXISTS`, `WITH ADMIN`
7-
//! - `introspection` — `SHOW TENANT[S]` by name / id + privilege gate
8-
//! - `grants` — `GRANT / REVOKE <perm> ON TENANT <name>`
6+
//! - `lifecycle` — CREATE / DROP TENANT, `IF [NOT] EXISTS`, `WITH ADMIN`
7+
//! - `introspection` — `SHOW TENANT[S]` by name / id + privilege gate
8+
//! - `grants` — `GRANT / REVOKE <perm> ON TENANT <name>`
9+
//! - `name_resolution` — DROP/ALTER/PURGE TENANT by name (mirroring CREATE/SHOW)
910
//!
1011
//! The entry file stays thin so future additions land in the topical
1112
//! submodule rather than bloating one ~500-line test file.
@@ -18,3 +19,5 @@ mod grants;
1819
mod introspection;
1920
#[path = "pgwire_auth_tenants/lifecycle.rs"]
2021
mod lifecycle;
22+
#[path = "pgwire_auth_tenants/name_resolution.rs"]
23+
mod name_resolution;
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
3+
//! Name-based tenant reference resolution on the DROP / ALTER / PURGE
4+
//! TENANT paths.
5+
//!
6+
//! Verifies that the legacy `DROP TENANT <id>` form keeps working and that
7+
//! a tenant name (bare or single-quoted) resolves to the same id via the
8+
//! shared `resolve_tenant_ref` helper, mirroring the already-shipped
9+
//! `CREATE TENANT <name>` / `SHOW TENANT <name>` paths.
10+
11+
use crate::common::pgwire_auth_helpers::{ddl_err, ddl_ok, make_state_with_catalog, superuser};
12+
13+
// ─── DROP TENANT by name ─────────────────────────────────────────────────────
14+
15+
/// `DROP TENANT <id>` (numeric) — regression that the legacy form still
16+
/// works after the resolver refactor.
17+
#[tokio::test]
18+
async fn drop_tenant_by_numeric_id_still_works() {
19+
let state = make_state_with_catalog();
20+
let su = superuser();
21+
22+
ddl_ok(&state, &su, "CREATE TENANT acme_drop_num ID 7142").await;
23+
ddl_ok(&state, &su, "DROP TENANT 7142").await;
24+
}
25+
26+
/// `DROP TENANT <name>` — the new path; name resolves to the catalog id.
27+
#[tokio::test]
28+
async fn drop_tenant_by_bare_name() {
29+
let state = make_state_with_catalog();
30+
let su = superuser();
31+
32+
ddl_ok(&state, &su, "CREATE TENANT acme_drop_name ID 7143").await;
33+
ddl_ok(&state, &su, "DROP TENANT acme_drop_name").await;
34+
}
35+
36+
/// `DROP TENANT '<name>'` — single-quoted name, matches the AST
37+
/// `TenantSelector` behavior on CREATE/SHOW.
38+
#[tokio::test]
39+
async fn drop_tenant_by_quoted_name() {
40+
let state = make_state_with_catalog();
41+
let su = superuser();
42+
43+
ddl_ok(&state, &su, "CREATE TENANT acme_drop_quoted ID 7144").await;
44+
ddl_ok(&state, &su, "DROP TENANT 'acme_drop_quoted'").await;
45+
}
46+
47+
/// `DROP TENANT <unknown_name>` without `IF EXISTS` errors with `42704`.
48+
#[tokio::test]
49+
async fn drop_tenant_unknown_name_without_if_exists_errors() {
50+
let state = make_state_with_catalog();
51+
let su = superuser();
52+
53+
let err = ddl_err(&state, &su, "DROP TENANT no_such_tenant").await;
54+
assert!(
55+
err.contains("does not exist") && err.contains("42704"),
56+
"expected 42704/does not exist, got: {err}"
57+
);
58+
}
59+
60+
/// `DROP TENANT IF EXISTS <unknown_name>` is a no-op success — parallels the
61+
/// `IF EXISTS <unknown_id>` semantics.
62+
#[tokio::test]
63+
async fn drop_tenant_if_exists_unknown_name_is_noop() {
64+
let state = make_state_with_catalog();
65+
let su = superuser();
66+
67+
ddl_ok(&state, &su, "DROP TENANT IF EXISTS no_such_tenant").await;
68+
}
69+
70+
/// `DROP TENANT ''` (empty quoted name) → `42601` syntax error.
71+
#[tokio::test]
72+
async fn drop_tenant_empty_name_errors() {
73+
let state = make_state_with_catalog();
74+
let su = superuser();
75+
76+
let err = ddl_err(&state, &su, "DROP TENANT ''").await;
77+
assert!(
78+
err.contains("42601") && err.contains("numeric id or a tenant name"),
79+
"expected 42601 empty-name error, got: {err}"
80+
);
81+
}
82+
83+
// ─── ALTER TENANT by name ────────────────────────────────────────────────────
84+
85+
/// `ALTER TENANT <id> SET QUOTA ...` — regression: numeric form still works.
86+
#[tokio::test]
87+
async fn alter_tenant_by_numeric_id_still_works() {
88+
let state = make_state_with_catalog();
89+
let su = superuser();
90+
91+
ddl_ok(&state, &su, "CREATE TENANT acme_alter_num ID 7145").await;
92+
ddl_ok(
93+
&state,
94+
&su,
95+
"ALTER TENANT 7145 SET QUOTA max_qps = 250",
96+
)
97+
.await;
98+
}
99+
100+
/// `ALTER TENANT <name> SET QUOTA ...` — name resolves to id.
101+
#[tokio::test]
102+
async fn alter_tenant_by_name() {
103+
let state = make_state_with_catalog();
104+
let su = superuser();
105+
106+
ddl_ok(&state, &su, "CREATE TENANT acme_alter_name ID 7146").await;
107+
ddl_ok(
108+
&state,
109+
&su,
110+
"ALTER TENANT acme_alter_name SET QUOTA max_qps = 250",
111+
)
112+
.await;
113+
}
114+
115+
/// `ALTER TENANT <unknown_name> SET QUOTA ...` errors with `42704`.
116+
#[tokio::test]
117+
async fn alter_tenant_unknown_name_errors() {
118+
let state = make_state_with_catalog();
119+
let su = superuser();
120+
121+
let err = ddl_err(
122+
&state,
123+
&su,
124+
"ALTER TENANT no_such_tenant SET QUOTA max_qps = 250",
125+
)
126+
.await;
127+
assert!(
128+
err.contains("does not exist") && err.contains("42704"),
129+
"expected 42704/does not exist, got: {err}"
130+
);
131+
}

0 commit comments

Comments
 (0)