Commit 7cd7dbb
committed
Address review comments on submodule support
Submodule tables were stored under the wrong name
-------------------------------------------------
`create_table_from_def_with_prefix` set a submodule table's canonical name from
its *accessor* name and then dropped the alias entirely. Accessor names exist
for client codegen; the host must never key on them. This made the host resolve
submodule tables by a codegen-only name and left `st_table_accessor` with no row
to recover the real accessor from.
The view path already did this correctly and said so in a comment, so the two
disagreed with each other; codegen had the same split, emitting
`prefix + accessor_name` as a table's wire name but `prefix + name` for views.
It was invisible only because accessor == canonical in the test fixture.
Submodule tables are now stored as `prefix + canonical name`, with the accessor
name kept as a namespaced alias, matching root tables. `check_compatible` goes
back to an exact name match rather than accepting either name.
`submodule_table_is_stored_under_canonical_name` covers this with a
case-converted name, where the two actually differ.
Local names were recovered by splitting on the last `.`
-------------------------------------------------------
`check_compatible` recovered an index, constraint, sequence or schedule function's
local name with `rsplit('.')`. A V9 sub-object name may itself contain dots -- the
`wacky_names` test builds `"wacky.index()"` -- so the last segment is not the local
name, and republishing such a module failed with "Index 0 not found in definition".
Master compared the stored name to `def.name` outright and was unaffected.
Now that every def records the namespace it is mounted under, the local name is
recovered by stripping exactly that prefix, via `NamespacePath::strip_from`, which
borrows rather than allocating. `wacky_names` covers the round-trip.
The table-name check was loose in the same place: it compared only the last segment,
so a table stored as `lib.my_table` validated against a root def named `my_table`.
It now checks the whole name, and `check_compatible_submodule_table_name` covers a
def actually mounted in `lib`.
The related *panic* is gone too: `auto_migrate_indexes` and friends built keys with
`Identifier::new(name).expect("names in a validated ModuleDef are valid identifiers")`,
which aborts outright on `"wacky.index()"`. Keying by `(namespace, name)` removes the
need to parse the name at all.
`ModuleDefLookup::key()` restored
---------------------------------
That naming split is why `key()` had to go: a table had two candidate keys.
With it fixed, each def records the `NamespacePath` its module is mounted under
(stamped by `ModuleDef::apply_namespace` once the module tree is assembled) and
its key is `(namespace, name)` -- a `Copy` pair, so `AutoMigrateStep` and
`AutoMigratePrecheck` go back to borrowing `Key<'def>` as they do on master.
The local name is stored once; the qualified form is built where it is actually
needed, which is the few places that hand a name to the database. That is 19
`format!`s at the same migration and table-creation sites this PR already had
them. They are per migration step and per table, never per row, and
`table_id_from_name` already allocates an owned `AlgebraicValue::String` to probe
the index, so the marginal cost is one short string on top of one that was
unavoidable.
Sub-objects (indexes, sequences, constraints) key on `(namespace, local name)`
rather than a dot-joined name, because a V9 sub-object name may itself contain
dots -- `wacky_names` covers exactly that -- and `stored_in_table_def` already
maps a sub-object to its table, so the key does not need to name the table.
Schedules are 1:1 with tables, so a table's key identifies its schedule.
`ModuleDef` now knows the path it is mounted under, so a namespaced key resolves
relative to whichever module it is looked up in -- from the root, or from the
submodule that owns the def.
This replaces the `find_*_by_full_name(&str)` helpers, which scanned every table
in the tree and `format!`ed each candidate, with `find_table`, `find_view` and
`find_storing_table`, which take keys and walk the submodule tree by segment.
`ensure_same_schema` filtered submodule steps with `name.contains('.')`; it now
asks whether the namespace is empty.
Type safety for namespaced names
--------------------------------
`RawIdentifier` was used for dot-delimited names, which is wrong: a name
containing `.` can never be validated into one `Identifier`. Add
`RawNamespacedIdentifier` and use it where a name may carry a namespace:
`TableName`, `SqlIdent`, index/constraint/sequence names and their `st_*` rows,
and query-planner relvar names. `ScheduleSchema::function_name` becomes a
`NamespacedIdentifier`.
`ReducerName` becomes fully qualified, so `local()` gives the name within its own
module and the `Deref<str>` gives the wire name. Previously it held a single
`Identifier`, so converting it to a `NamespacedIdentifier` yielded a one-segment
name -- `verify_token`, not `myauth.verify_token`.
The v1/v2 websocket message types are deliberately left as `RawIdentifier`.
They have always carried the table name as an opaque string and this feature
does not change that, so the narrowing happens at the boundary in `core` rather
than churning a published protocol crate.
This removes the `rsplit('.')` "bare name" hacks in `schema.rs` in favour of
`local_name()`, and namespace prefixing now goes through typed
`NamespacePath::{join, join_raw, join_namespaced}` instead of `format!` into a
`RawIdentifier`.
`Identifier::new_assume_valid` is renamed to `new_unsafe_assume_valid` so it
reads as the escape hatch it is; the schedule-name call site that used it to
smuggle a dotted name into an `Identifier` is gone.
Note this makes `TxDataTableEntry` and `MutTxId` grow, which the static size
assertions record. `TableName` and `ReducerName` wrap a `NamespacedIdentifier`,
which stores both the segments and their joined rendering. Those names genuinely
are qualified -- they are the database and wire identities -- so shrinking them
would mean changing how `NamespacedIdentifier` itself is represented.
Host boundary
-------------
`InstanceOp::name()` returned an owned `NamespacedIdentifier`, so every call
cloned; it returns a reference again. `ProcedureOp`/`HttpHandlerOp` build theirs
once at construction and `ReducerOp` borrows its `ReducerName`. Only
`start_funcall`, which takes the name by value, still clones. `start_funcall`
itself took `&str`/`RawIdentifier` and now takes a `NamespacedIdentifier` in v8,
wasmtime and `wasm_instance_env`.
A submodule reducer is now named by its qualified name in logs and metrics
rather than its bare local name.
Also fixes stale-view backing-table recreation, which iterated only root views
and looked them up in `st_view` by local name, so submodule views were never
repaired.
TypeScript
----------
- camelCase throughout `lib_submodule.ts`, `index.ts` and the submodules doc
- drop the `Anonymous extends true ? ... : ...` generic on `registerView` and
split it into `registerView` / `registerAnonymousView`, removing the
`as unknown as ViewFn<any, any, any>` cast; what remains is a single
documented schema-erasure assertion per flavour
- generated internals use `__`-prefixed names (`__qb`, `__reducerAccessors`,
`__procedureAccessors`), matching the rest of the emitted code
- the docs' subscription example used `addQuery`, which this branch removed
The camelCase rename is to the TypeScript *exports*; the wire name is the
canonical snake_case form and does not change. The docs now spell that out,
since the HTTP and CLI paths take the canonical name while the generated
bindings expose the camelCase accessor.
`submodule_reducer_wire_name_is_qualified_once` pins the codegen side, as no
snapshot fixture mounts a submodule.
Other
-----
- restore the step-ordering note inside `AutoMigrateStep` where it was
- restore the named bindings in `successful_auto_migration` to shrink the diff
- drop an unrelated `sender` -> `s` rename in the view-call path
- `RawNamespacedIdentifier::segments` collected a `Vec` per call to be an
`ExactSizeIterator`; no caller reads the size, so it just yields the `split`
- the docs said a submodule view is reachable as `<namespace>.<viewName>` in SQL.
SQL takes the canonical snake_case name; only the generated bindings expose the
camelCase accessor1 parent 0dde407 commit 7cd7dbb
48 files changed
Lines changed: 1733 additions & 1088 deletions
File tree
- crates
- bench/src
- bindings-typescript/src/server
- cli/src/subcommands
- codegen
- src
- tests
- core/src
- client
- host
- v8
- wasm_common
- wasmtime
- subscription
- datastore/src
- locking_tx_datastore
- engine/src
- expr/src
- sats/src
- schema
- src
- auto_migrate
- def
- validate
- tests
- sql-parser/src
- ast
- parser
- table/src
- testing/tests
- docs/docs/00200-core-concepts/00100-databases
- modules/module-test-ts/src
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
| 72 | + | |
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| |||
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
76 | | - | |
| 76 | + | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| |||
194 | 194 | | |
195 | 195 | | |
196 | 196 | | |
197 | | - | |
198 | 197 | | |
199 | 198 | | |
200 | 199 | | |
201 | 200 | | |
202 | 201 | | |
203 | 202 | | |
204 | | - | |
205 | 203 | | |
206 | 204 | | |
207 | | - | |
208 | | - | |
209 | | - | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
210 | 248 | | |
211 | 249 | | |
212 | 250 | | |
213 | 251 | | |
214 | 252 | | |
215 | 253 | | |
216 | 254 | | |
217 | | - | |
218 | | - | |
219 | 255 | | |
220 | 256 | | |
221 | 257 | | |
| |||
255 | 291 | | |
256 | 292 | | |
257 | 293 | | |
258 | | - | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
259 | 298 | | |
260 | | - | |
261 | | - | |
262 | | - | |
263 | | - | |
264 | | - | |
265 | 299 | | |
266 | 300 | | |
267 | 301 | | |
268 | 302 | | |
269 | 303 | | |
270 | | - | |
271 | | - | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
272 | 323 | | |
273 | 324 | | |
274 | 325 | | |
275 | | - | |
| 326 | + | |
276 | 327 | | |
277 | 328 | | |
278 | 329 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
285 | 285 | | |
286 | 286 | | |
287 | 287 | | |
288 | | - | |
| 288 | + | |
289 | 289 | | |
290 | 290 | | |
291 | 291 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
320 | 320 | | |
321 | 321 | | |
322 | 322 | | |
323 | | - | |
| 323 | + | |
324 | 324 | | |
325 | 325 | | |
326 | 326 | | |
| |||
377 | 377 | | |
378 | 378 | | |
379 | 379 | | |
380 | | - | |
| 380 | + | |
| 381 | + | |
381 | 382 | | |
382 | 383 | | |
383 | 384 | | |
| |||
593 | 594 | | |
594 | 595 | | |
595 | 596 | | |
596 | | - | |
| 597 | + | |
597 | 598 | | |
598 | 599 | | |
599 | 600 | | |
600 | 601 | | |
601 | 602 | | |
602 | | - | |
| 603 | + | |
603 | 604 | | |
604 | 605 | | |
605 | 606 | | |
606 | 607 | | |
607 | | - | |
| 608 | + | |
608 | 609 | | |
609 | 610 | | |
610 | 611 | | |
| |||
622 | 623 | | |
623 | 624 | | |
624 | 625 | | |
625 | | - | |
| 626 | + | |
626 | 627 | | |
627 | 628 | | |
628 | 629 | | |
| |||
631 | 632 | | |
632 | 633 | | |
633 | 634 | | |
634 | | - | |
| 635 | + | |
635 | 636 | | |
636 | 637 | | |
637 | | - | |
| 638 | + | |
638 | 639 | | |
639 | 640 | | |
640 | 641 | | |
| |||
652 | 653 | | |
653 | 654 | | |
654 | 655 | | |
655 | | - | |
| 656 | + | |
656 | 657 | | |
657 | 658 | | |
658 | 659 | | |
659 | 660 | | |
660 | 661 | | |
661 | | - | |
| 662 | + | |
662 | 663 | | |
663 | 664 | | |
664 | | - | |
| 665 | + | |
665 | 666 | | |
666 | 667 | | |
667 | 668 | | |
| |||
1328 | 1329 | | |
1329 | 1330 | | |
1330 | 1331 | | |
1331 | | - | |
1332 | | - | |
1333 | | - | |
| 1332 | + | |
| 1333 | + | |
| 1334 | + | |
| 1335 | + | |
| 1336 | + | |
1334 | 1337 | | |
1335 | 1338 | | |
1336 | 1339 | | |
| |||
1413 | 1416 | | |
1414 | 1417 | | |
1415 | 1418 | | |
1416 | | - | |
| 1419 | + | |
1417 | 1420 | | |
1418 | 1421 | | |
1419 | 1422 | | |
| |||
1442 | 1445 | | |
1443 | 1446 | | |
1444 | 1447 | | |
1445 | | - | |
| 1448 | + | |
1446 | 1449 | | |
1447 | 1450 | | |
1448 | 1451 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
20 | 21 | | |
21 | 22 | | |
22 | 23 | | |
| |||
608 | 609 | | |
609 | 610 | | |
610 | 611 | | |
611 | | - | |
| 612 | + | |
612 | 613 | | |
613 | 614 | | |
614 | 615 | | |
| |||
621 | 622 | | |
622 | 623 | | |
623 | 624 | | |
624 | | - | |
| 625 | + | |
625 | 626 | | |
626 | 627 | | |
627 | 628 | | |
| |||
639 | 640 | | |
640 | 641 | | |
641 | 642 | | |
642 | | - | |
| 643 | + | |
643 | 644 | | |
644 | 645 | | |
645 | 646 | | |
| |||
652 | 653 | | |
653 | 654 | | |
654 | 655 | | |
655 | | - | |
| 656 | + | |
656 | 657 | | |
657 | 658 | | |
658 | 659 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
25 | 24 | | |
26 | 25 | | |
27 | 26 | | |
28 | 27 | | |
29 | 28 | | |
| 29 | + | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | | - | |
| 51 | + | |
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| |||
246 | 246 | | |
247 | 247 | | |
248 | 248 | | |
249 | | - | |
| 249 | + | |
250 | 250 | | |
251 | 251 | | |
252 | 252 | | |
| |||
0 commit comments