Skip to content

Commit 5e83ce2

Browse files
committed
don't pass qualified names around
1 parent e34b52c commit 5e83ce2

54 files changed

Lines changed: 881 additions & 844 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

plans/GLOSSARY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ bumping schema_epoch; barrier and shutdown path
194194
([emitter.md](emitter.md))
195195

196196
**forward declaration**`config_table` row whose relation doesn't
197-
exist yet; parked keyed on qualified name, materialized when matching
197+
exist yet; parked keyed on `(namespace, relname)`, materialized when matching
198198
CREATE TABLE arrives ([config.md](config.md))
199199

200200
**FPI** — full-page image on a WAL block ref; `restore_block_image`

plans/config.md

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ daemon — preserving walshadow's read-only-source posture. Four tables:
6868
`drop_table_strategy`
6969
- `config_namespace` — key `namespace`: `target_database`, `auto_create`,
7070
`drop_table_strategy`
71-
- `config_table` — key `(namespace, relname)`: `target` (`"<db>.<table>"`),
71+
- `config_table` — key `(namespace, relname)`: `target_database`,
72+
`target_table` (each NULL = derived: namespace default / source relname),
7273
`replicate`, `initial_load` (`none`, `copy`, `base_backup`, `object_store`).
73-
Text key, not relfilenode, rfn is unknown at row-insert time for
74+
Name key, not relfilenode, rfn is unknown at row-insert time for
7475
forward-declared tables
7576
- `config_column` — key `(namespace, relname, attname)`: `target_type`
7677

@@ -85,8 +86,8 @@ No `config_decoder` task, no relfilenode filter. Config-table writes ride the
8586
normal heap-decode path and are intercepted in
8687
[`BufferingDecoderSink::on_record`](../src/xact_buffer.rs) after decode, before
8788
routing to CH: a write is a config write when its resolved descriptor has
88-
`namespace_name == <schema>` and `ConfigTableKind::from_relname(name)` matches
89-
one of the four tables. Detection by resolved qualified name is **rotation-proof
89+
`rel_name.namespace == <schema>` and `ConfigTableKind::from_relname(name)` matches
90+
one of the four tables. Detection by resolved relation name is **rotation-proof
9091
for free** — TRUNCATE / VACUUM FULL / rewrite rotates the relfilenode but the
9192
decode path re-resolves every descriptor, so the name still matches, with no
9293
frozen filter to refetch. Config writes never reach CH (the implicit namespace
@@ -183,10 +184,11 @@ compatibility needs the descriptor, so that check falls back at plan build —
183184
(`ConfigResolver::rejections`) and log at WARN. Validation runs at merge, not
184185
at decode.
185186

186-
`config_table.target` overrides the destination only of a table already mapped by
187-
TOML (which carries the column projection); a row for an unmapped table would
188-
need column auto-derivation, so it is skipped with a WARN rather than emitting a
189-
column-less INSERT.
187+
`config_table.target_database` / `target_table` override the destination only
188+
of a table already mapped by TOML (which carries the column projection); a row
189+
for an unmapped table would need column auto-derivation, so it is skipped with
190+
a WARN rather than emitting a column-less INSERT. NULL leaves that part of the
191+
destination unchanged.
190192

191193
## Column overrides
192194

plans/emitter.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -259,20 +259,26 @@ without walshadow having to track which rows already landed
259259

260260
## Mapping config
261261

262-
`EmitterConfig::tables` parses from TOML `[table."<src>"]` blocks:
262+
`EmitterConfig::tables` parses from TOML `[table.<namespace>.<relname>]`
263+
blocks (two key levels; names with weird characters quote per TOML key
264+
rules). Destination parts stay separate — `TableTarget { database, table }`
265+
joins only at SQL construction (`TableTarget::sql`); `target_database`
266+
defaults to the namespace override else `[ch] database`, `target_table` to
267+
the source relname:
263268

264269
```toml
265-
[table."public.foo"]
270+
[table.public.foo]
266271
replicate = true
267272
initial_load = "none"
268-
target = "default.foo"
273+
target_database = "default"
274+
target_table = "foo"
269275
columns = [
270276
{ attnum = 1, target = "id", type = "UInt64" },
271277
{ attnum = 2, target = "name", type = "Nullable(String)" },
272278
]
273279
```
274280

275-
`MappingHandle = Arc<tokio::sync::RwLock<HashMap<String, TableMapping>>>`
281+
`MappingHandle = Arc<tokio::sync::RwLock<HashMap<RelName, TableMapping>>>`
276282
is the live handle the decode pool consults per row. Handle is
277283
cloneable; daemon's SIGHUP task swaps whole inner `HashMap`. Routing
278284
picks up the swap immediately; the batcher's cached `TableEncoder`
@@ -342,7 +348,7 @@ table:
342348
| `Added { desc }` | `CREATE TABLE IF NOT EXISTS` (in the namespace's `target_database`, else global default) when namespace `auto_create = true` and no pre-pinned mapping. Auto-derives `TableMapping` against that same database post-success so subsequent rows ship against the new table. A mapped rel under strategy = drop instead re-creates its dest from the mapping (`render_create_table_from_mapping`) — dest lifecycle follows source DDL, so create → drop → create round-trips; `IF NOT EXISTS` no-ops when the dest still stands |
343349
| `Changed { diff }` | `ALTER TABLE … RENAME COLUMN` first (so position-match diffs don't trip into drop+add), then `ALTER TABLE … ADD COLUMN IF NOT EXISTS` per added attnum, then `ALTER TABLE … DROP COLUMN IF EXISTS` per dropped attnum |
344350
| `Changed.type_changes` | rejected, logged, `stats.type_changes_rejected += n`. Operator handles via manual CH migration |
345-
| `Dropped { qualified_name }` | gated on the namespace's `DropTableStrategy` (`drop_strategy_for`, else global): `Retain` (default) skips silently, `Warn` skips at WARN, `Drop` runs `DROP TABLE IF EXISTS` |
351+
| `Dropped { rel_name }` | gated on the namespace's `DropTableStrategy` (`drop_strategy_for`, else global): `Retain` (default) skips silently, `Warn` skips at WARN, `Drop` runs `DROP TABLE IF EXISTS` |
346352

347353
`render_create_table` builds CREATE off descriptor: attributes through
348354
`type_bridge::map`, PK columns first in `ORDER BY` (else `_lsn`
@@ -375,7 +381,7 @@ before its first `ALTER` records the post-ALTER shape as `Added`, which
375381
`apply_added` skips for pinned dests (operator-managed CH) → CH stays a
376382
column behind.
377383

378-
`ShadowCatalog::seed_baseline(qualified_names)` warms `prev_known` for
384+
`ShadowCatalog::seed_baseline(rel_names)` warms `prev_known` for
379385
every pinned relation before `subscribe()` so the cache never decides the
380386
branch: `bin/stream.rs` calls it after preflight / before
381387
`START_REPLICATION` over `cfg.tables.keys()` (the inproc harness mirrors

plans/future/runtime_config_from_pg.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,8 @@ surgical when the xact also carries changes to keep.
138138

139139
## Per-table opt-in and initial-load path
140140

141-
The base `config_table` ([../config.md](../config.md)) carries only `target`.
141+
The base `config_table` ([../config.md](../config.md)) carries only the
142+
target columns.
142143
This adds two columns — `replicate` (bool, doubles as the inclusion switch)
143144
and `initial_load` (text mode: `'none'`, `'copy'`, `'base_backup'`,
144145
`'object_store'`; SQL NULL means omitted) — collapsing three intents into one
@@ -160,8 +161,8 @@ inspects `replicate` + `initial_load` + catalog state to dispatch:
160161
| `replicate=t` | no (forward-decl) | n/a | hold row, materialize when CREATE TABLE for matching qualname arrives via catalog applicator |
161162
| `replicate=f` | yes | n/a | inclusion-list remove; mid-stream exclusion drains in-flight rows then halts further emission |
162163

163-
Keyed on **qualified name**, not rfn. Resolver maintains a
164-
`pending_decl: HashMap<QualifiedName, ConfigTable>` populated from rows whose
164+
Keyed on **relation name** (`(namespace, relname)` pair), not rfn. Resolver
165+
maintains a `pending_decl: HashMap<RelName, TableRow>` populated from rows whose
165166
target rfn doesn't exist. The catalog applicator notifies the resolver on each
166167
new rel; resolver pops the matching pending entry and registers the rfn↔config
167168
binding. Stale entries (rel never created, or dropped and recreated under a
@@ -340,8 +341,8 @@ subcommand walking the three layers.
340341
during the COPY win by `commit_lsn > S`. Source state == CH state once WAL apply
341342
passes `P_hi`. Variant: mutate a row mid-backfill and assert CH reflects the
342343
mutation, not the COPY baseline
343-
- **Forward-decl.** Operator inserts `config_table (qname = "app.new_table",
344-
replicate=true)` for a table that doesn't exist. Resolver parks it in
344+
- **Forward-decl.** Operator inserts `config_table (namespace = 'app',
345+
relname = 'new_table', replicate = true)` for a table that doesn't exist. Resolver parks it in
345346
`pending_decl`. Source runs `CREATE TABLE app.new_table (...)`; the catalog
346347
applicator notifies the resolver, the pending row resolves, subsequent inserts
347348
land on CH under the declared config

plans/shadow.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,9 @@ backoff policy varyable per call site
171171

172172
What catalog produces per relation:
173173

174-
- `rfn: RelFileNode`, `oid: Oid`, `namespace_oid`, `namespace_name`,
175-
`name`, `qualified_name: Arc<str>` (pre-formatted for hot-path
176-
routing)
174+
- `rfn: RelFileNode`, `oid: Oid`, `namespace_oid`, `rel_name: RelName`
175+
(structured `{ namespace, name }` pair, `Arc<str>` parts for hot-path
176+
routing; joined only at SQL interpolation / `Display`)
177177
- `kind` (`pg_class.relkind`: `'r'` table / `'p'` partitioned / etc),
178178
`persistence` (`'p'` / `'u'` / `'t'`)
179179
- `replident: ReplIdent` — resolved from `pg_class.relreplident`
@@ -234,7 +234,7 @@ Variants (see diagram legend for trigger → DDL mapping):
234234
`added_columns`, `dropped_columns`, `renamed_columns`, `type_changes`.
235235
Renames detected by attnum-match + name-diff heuristic; PG's `RENAME
236236
COLUMN` keeps attnum intact, natural case lands here
237-
- `Dropped { oid, qualified_name }``emit_dropped(oid)` from
237+
- `Dropped { oid, rel_name }``emit_dropped(oid)` from
238238
`pg_class_decoder` heap_delete branch, or `sweep_dropped` poll (at
239239
the dropping xact's commit, `PendingSweeps`-gated) for catalogs with
240240
`relreplident = 'n'` where heap_delete carries no old tuple to

sql/runtime_config_install.sql

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,18 @@ CREATE TABLE IF NOT EXISTS :"walshadow_schema".config_namespace (
3737
drop_table_strategy text -- overrides config_global for this namespace
3838
);
3939

40-
-- Per-relation destination mapping, keyed on qualified name (text, not
40+
-- Per-relation destination mapping, keyed on (namespace, relname) (not
4141
-- relfilenode: rfn is unknown at row-insert time for forward-declared tables).
4242
CREATE TABLE IF NOT EXISTS :"walshadow_schema".config_table (
43-
namespace text NOT NULL,
44-
relname text NOT NULL,
45-
target text, -- "<database>.<table>" on ClickHouse
46-
replicate boolean, -- inclusion switch: true opt-in, false
43+
namespace text NOT NULL,
44+
relname text NOT NULL,
45+
target_database text, -- ClickHouse database; NULL derives from
46+
-- config_namespace.target_database / TOML
47+
target_table text, -- ClickHouse table; NULL derives from relname
48+
replicate boolean, -- inclusion switch: true opt-in, false
4749
-- opt-out, NULL leaves scope unchanged
48-
-- (target override only, as before)
49-
initial_load text, -- one-time backfill mode for pre-opt-in
50+
-- (target override only)
51+
initial_load text, -- one-time backfill mode for pre-opt-in
5052
-- rows: 'none' | 'copy' | 'base_backup'
5153
-- | 'object_store'; NULL means omitted
5254
PRIMARY KEY (namespace, relname)
@@ -64,8 +66,10 @@ CREATE TABLE IF NOT EXISTS :"walshadow_schema".config_column (
6466
-- Additive upgrade: columns introduced after the initial config_table shape.
6567
-- CREATE TABLE IF NOT EXISTS above no-ops on an existing install, so an
6668
-- upgrading deployment re-runs this to gain the columns the newer daemon reads.
67-
ALTER TABLE :"walshadow_schema".config_table ADD COLUMN IF NOT EXISTS replicate boolean;
68-
ALTER TABLE :"walshadow_schema".config_table ADD COLUMN IF NOT EXISTS initial_load text;
69+
ALTER TABLE :"walshadow_schema".config_table ADD COLUMN IF NOT EXISTS replicate boolean;
70+
ALTER TABLE :"walshadow_schema".config_table ADD COLUMN IF NOT EXISTS initial_load text;
71+
ALTER TABLE :"walshadow_schema".config_table ADD COLUMN IF NOT EXISTS target_database text;
72+
ALTER TABLE :"walshadow_schema".config_table ADD COLUMN IF NOT EXISTS target_table text;
6973

7074
-- REPLICA IDENTITY FULL logs the complete old-row image on UPDATE/DELETE, so a
7175
-- DELETE always carries the key columns the decoder reads (namespace/relname/

src/backfill_bootstrap.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use crate::backup_page_walk::{
3838
use crate::backup_sink::{CatalogFilenodes, DiskLanderSink, DiskLanderStats, MultiplexSink};
3939
use crate::backup_source::{BackupSink, BackupSource, EndInfo, StartInfo};
4040
use crate::decoder_sink::TupleObserver;
41-
use crate::shadow_catalog::{RelAttr, RelDescriptor, ReplIdent, parse_array_one_element};
41+
use crate::shadow_catalog::{RelAttr, RelDescriptor, RelName, ReplIdent, parse_array_one_element};
4242

4343
#[derive(Debug, Clone)]
4444
pub struct BootstrapConfig {
@@ -279,14 +279,11 @@ pub async fn seed_catalog_from_source(client: &Client) -> Result<CatalogMap> {
279279
};
280280
let replident = fetch_replident(client, replident_char, oid).await?;
281281
let attributes = fetch_attributes(client, oid).await?;
282-
let qualified_name = RelDescriptor::build_qualified_name(&namespace_name, &name);
283282
let desc = RelDescriptor {
284283
rfn,
285284
oid,
286285
namespace_oid,
287-
namespace_name,
288-
name,
289-
qualified_name,
286+
rel_name: RelName::new(&namespace_name, &name),
290287
kind,
291288
persistence,
292289
replident,

src/backfill_staging.rs

Lines changed: 15 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -25,64 +25,20 @@ use clickhouse_c::{AsyncClient, Block, Event};
2525

2626
use crate::backup_backfill::BackupRequest;
2727
use crate::ch_emitter::{
28-
EmitterConfig, EmitterError, MappingHandle, RetryConfig, TableMapping, connect_client,
29-
drain_to_end_of_stream, is_retryable, quote_ident,
28+
EmitterConfig, EmitterError, MappingHandle, RetryConfig, TableMapping, TableTarget,
29+
connect_client, drain_to_end_of_stream, is_retryable, quote_ident,
3030
};
31+
use crate::shadow_catalog::RelName;
3132

3233
/// `orders` loads into `orders__wsstg`; deterministic so a retry or boot
3334
/// recovery finds the prior attempt's table
3435
pub const STAGING_SUFFIX: &str = "__wsstg";
3536

36-
/// Split a mapping target into unquoted `(database, table)`. Accepts the
37-
/// opt-in derived shape (`` `db`.`t` ``, doubled-backtick escapes) and bare
38-
/// TOML shapes (`db.t`, `t`); a db-less target lands in `default_db`.
39-
pub fn parse_target(target: &str, default_db: &str) -> Option<(String, String)> {
40-
let mut parts: Vec<String> = Vec::new();
41-
if target.contains('`') {
42-
let mut it = target.chars().peekable();
43-
loop {
44-
if it.next()? != '`' {
45-
return None;
46-
}
47-
let mut cur = String::new();
48-
loop {
49-
match it.next()? {
50-
'`' if it.peek() == Some(&'`') => {
51-
it.next();
52-
cur.push('`');
53-
}
54-
'`' => break,
55-
c => cur.push(c),
56-
}
57-
}
58-
parts.push(cur);
59-
match it.next() {
60-
None => break,
61-
Some('.') => continue,
62-
Some(_) => return None,
63-
}
64-
}
65-
} else {
66-
match target.rsplit_once('.') {
67-
Some((db, t)) => parts.extend([db.to_owned(), t.to_owned()]),
68-
None => parts.push(target.to_owned()),
69-
}
70-
}
71-
match parts.len() {
72-
1 => Some((default_db.to_owned(), parts.pop()?)),
73-
2 => {
74-
let t = parts.pop()?;
75-
Some((parts.pop()?, t))
76-
}
77-
_ => None,
78-
}
79-
}
80-
8137
/// One rel's swap identities. `database`/`table` are the unquoted
8238
/// destination parts; `s_lsn` drives the copy-back filter.
8339
#[derive(Debug, Clone)]
8440
pub struct StagingRel {
85-
pub qname: String,
41+
pub rel: RelName,
8642
pub database: String,
8743
pub table: String,
8844
pub s_lsn: u64,
@@ -127,37 +83,31 @@ pub async fn prepare(
12783
) -> Result<StagingPlan> {
12884
let mut sess = StagingSession::connect(emitter).await?;
12985
let live_map = live.read().await.clone();
130-
let mut staged: HashMap<String, TableMapping> = HashMap::new();
86+
let mut staged: HashMap<RelName, TableMapping> = HashMap::new();
13187
let mut rels = Vec::new();
13288
for r in reqs {
133-
let qname = r.desc.qualified_name.as_ref();
134-
let Some(m) = live_map.get(qname) else {
89+
let name = &r.desc.rel_name;
90+
let Some(m) = live_map.get(name) else {
13591
tracing::warn!(
13692
target: "walshadow::backfill_staging",
137-
qname,
93+
qname = %name,
13894
"no mapping at pass start; rows will skip",
13995
);
14096
continue;
14197
};
142-
let Some((database, table)) = parse_target(&m.target, &emitter.database) else {
143-
bail!(
144-
"backfill_staging: unparseable mapping target {:?} for {qname}",
145-
m.target
146-
);
147-
};
14898
let rel = StagingRel {
149-
qname: qname.to_owned(),
150-
database,
151-
table,
99+
rel: name.clone(),
100+
database: m.target.database.clone(),
101+
table: m.target.table.clone(),
152102
s_lsn: r.s_lsn,
153103
};
154104
sess.rebuild_staging(&rel)
155105
.await
156-
.with_context(|| format!("backfill_staging: rebuild staging for {qname}"))?;
106+
.with_context(|| format!("backfill_staging: rebuild staging for {name}"))?;
157107
staged.insert(
158-
qname.to_owned(),
108+
name.clone(),
159109
TableMapping {
160-
target: rel.staging_sql(),
110+
target: TableTarget::new(&rel.database, &rel.staging_table()),
161111
columns: m.columns.clone(),
162112
},
163113
);
@@ -409,37 +359,10 @@ fn sql_str(s: &str) -> String {
409359
mod tests {
410360
use super::*;
411361

412-
#[test]
413-
fn parse_target_handles_quoted_and_bare_shapes() {
414-
assert_eq!(
415-
parse_target("`db`.`orders`", "default"),
416-
Some(("db".into(), "orders".into()))
417-
);
418-
assert_eq!(
419-
parse_target("db.orders", "default"),
420-
Some(("db".into(), "orders".into()))
421-
);
422-
assert_eq!(
423-
parse_target("orders", "default"),
424-
Some(("default".into(), "orders".into()))
425-
);
426-
assert_eq!(
427-
parse_target("`orders`", "default"),
428-
Some(("default".into(), "orders".into()))
429-
);
430-
// Embedded dot + doubled backtick stay inside the quoted ident
431-
assert_eq!(
432-
parse_target("`d.b`.`or``ders`", "default"),
433-
Some(("d.b".into(), "or`ders".into()))
434-
);
435-
assert_eq!(parse_target("`db`.`t`.`x`", "default"), None);
436-
assert_eq!(parse_target("`unterminated", "default"), None);
437-
}
438-
439362
#[test]
440363
fn staging_rel_renders_sql_names() {
441364
let rel = StagingRel {
442-
qname: "public.orders".into(),
365+
rel: RelName::new("public", "orders"),
443366
database: "db".into(),
444367
table: "orders".into(),
445368
s_lsn: 0x5000,

src/backup_backfill.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -946,12 +946,8 @@ impl crate::decoder_sink::TupleObserver for ReplayRouter {
946946
self.commits_past_s += 1;
947947
return Ok(());
948948
}
949-
let Some(mapping) = crate::pipeline::lookup_mapping(
950-
&self.mapping,
951-
rel.qualified_name.as_ref(),
952-
&self.stats,
953-
)
954-
.await
949+
let Some(mapping) =
950+
crate::pipeline::lookup_mapping(&self.mapping, &rel.rel_name, &self.stats).await
955951
else {
956952
return Ok(());
957953
};

0 commit comments

Comments
 (0)