cache: skip a relation the schema cannot describe, and emit views #49
No reviewers
Labels
No labels
area:defold
area:docs
area:infra
area:protocol
area:testing
kind:bug
kind:chore
phase:breadth
phase:core
phase:hardening
phase:slice
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
yuris/spacetimedb-defold!49
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/42-views"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #42 — the last open 1.0 blocker.
table_defreturned{}for a relation the schema does not describe, sodef.columnswas nil,decode_row(encoded, nil)setcolumns = {}, the mapping loop ran zero times and returned{}. An empty row was stored and counted as an insert, with no error anywhere. Every field of every view row was silently discarded, reachable by an ordinarySELECT * FROM my_view.It was worse than lossy. Those fieldless rows were reachable through the ordinary read API:
count()reported a row that had no fields,iter()handed out a table where every access is nil, and — because a pk-less delete scans for an equal row andpairs({})never falsifies a match — a delete of a row the relation never held evicted the one it did hold. A consumer who registers nothing at all is therefore strictly better off now: the relation is simply empty, which is honest and inert.Both halves of the issue are done, deliberately.
The loud half — skip and report, not raise
apply_database_updateskips an undescribed relation whole and reports it incounts.undescribed, whichemit_rowsturns into anundescribedevent.Not a raise, and the reason is concrete: a raise aborts the frame's
tablesloop, so it would drop the rows of relations the consumer did describe and park the subscribe callback, repeating on every reconnect.handle_framehas nopcallat any ofapply_database_update's four call sites, so it would propagate into Defold's websocket callback and kill the connection because someone subscribed to a view.The loss a raise causes is also order-dependent, which is the strongest form of the argument. Replaying one frame containing an undescribed
ghostand a describeditem, against a patched cache that raises:itemrowsghostfirstitemfirstdb_update.tablesis ordered by the server, so a raise would lose described rows nondeterministically.This follows the
unhandled-event patternhandle_framealready uses for an unknown message tag — loud without being fatal — rather thancache.find's raise, which is a programmer error on a call the consumer controls.Discoverability: the report reaches the handle, not only
on_eventAn event is only audible if you registered
on_event, and the natural way to consume a view issubscribe_query({"SELECT * FROM my_view"}, cb). On that path the SDK affirmatively reported success —callback(nil, msg), handleapplied,is_active()true — while every row was dropped, andon_eventdefaults to a no-op with no logging facility anywhere instdb/.Those signals are all correct (the server really did apply the set) and
callback(err, msg)is frozen, so the report goes on the handle instead:sub.undescribedis the same{ { table, rows, reason } }list under the same name, written beforeon_appliedfires and rewritten on every apply — nil included, since the set re-applies after a reconnect and a stale report would be an accusation the handle could never withdraw. The legacyM.subscribereturns a bare request_id with no handle to carry it, and its docstring says so.The generator half
gen-schema.shresolvesmisc_exports[].View's return type to the view's own column order and emits it intotableswithview = true.Both return forms are emitted. The Array form is
{Array: {Ref: n}}; the Option form — a view yielding at most one row — declares the same kind of product through thesomevariant of a two-variant sum, so one helper resolves either and everything downstream, typespace seeding included, is identical. Verified end to end on a live 2.8.2:SELECT * FROM message_latestdelivers["second",2], positional in the view's declared(text, id)order, and decodes throughschema.new(gen.tables)totext = "second"andid = 2as a number. Reusing the underlying table's(id, sender, text)would have put the u64 intext. The dev module carries that view so the second resolution hop stays covered.A view's own pk is still absent: a View at that endpoint carries name, params,
is_public,is_anonymous,indexandreturn_type— no key of any kind, even when the module source declares one — so nopkis emitted rather than guessed, and a changed view row is a delete plus an insert instead of one update event.That ordering is the part that could have been worse than the raise, so it is measured. Frames 37–42 are a new capture block proving:
The tool itself is now checked, not just its output. Nothing re-derived
chat_schema.lua, so a generator resolving the wrong typespace entry — emitting a view with its underlying table's column order, the exact silent mis-mapping #42 exists to prevent — passed the whole Lua suite, which only ever reads the committed fixture.conformance.shregenerates the schema from the module it just published and diffs it against the committed copy, ignoring line 1's source URL whose port followsSTDB_PORT. That mutation now fails the run.dev-server.shno longer rewrites the fixture on every publish: a clobber can never detect drift, because it destroys the copy it would have to compare against.Verification
6 passed, 0 failed; stylua clean. Native and HTML5 conformance both PASS, including PHASE5's identity-keyed check, with the new schema diff green inside the native run.Reverting the guard fails three files by name. The review's mutation sweep found two survivors and both are closed: two undescribed relations in one frame are now pinned in
test_cache.lua(two entries, wire order, own row counts) and intest_connection.lua(two notices), soskipped[#skipped + 1]→skipped[1]fails both files; and the generator's wrong-ref mutation fails the conformance diff.On #42's acceptance wording
#42's Acceptance line asks for "a test that an unknown relation raises rather than storing an empty row." Nothing raises, and that is deliberate: #42's own Work bullet authorises the deviation — "should raise, or emit a distinct event" — and the table above is why the event is the better half of that latitude. Recorded here because the issue text will outlive the PR.
Frozen surface
undescribed, oncounts, as an event, and now on the Subscription handle: one concept, one name. It mirrorsunhandledfor an unknown message tag — same class of problem, same bare-adjective form.{ type, table, rows, reason }.rowsis a count, not a list: there is nothing to hand over, since the rows were never decoded, and it is the vocabularycountsalready uses forinsertsanddeletes. Documented as a count at both surfaces, because the siblingrowon an insert or delete event is a row.countsstays additive: with nothing skipped the keys are exactlydeletes, events, inserts, and no consumer anywhere iteratespairs(counts).Subscribing to a view stored one fieldless row per projection and counted each as an insert, with no error anywhere. `table_def` returned `{}` for an unknown relation, so `def.columns` was nil, and `protocol.decode_row` with no columns is total loss: `columns` defaults to `{}`, both of its loops become `ipairs({})`, and it returns `{}` for any input at all — a named readback row included. Reachable by an ordinary `SELECT * FROM my_view`, and the same failure class this project has spent #8, #17, #21, #26 and #46 eliminating. Downstream it was worse than empty. A pk-less `remove` scans for a row equal to the one being deleted field by field; `pairs({})` never falsifies the match, so the first candidate always won and a delete evicted the LAST row of the relation — even a delete of a row the relation never held. SKIP AND REPORT, NOT RAISE. `apply_database_update` now skips such a relation whole and records it in `counts.undescribed`, which `emit_rows` turns into an `undescribed` event. A raise would have aborted the frame's table loop: the relations listed after the undescribed one would never be applied, `counts` would never be returned, so `emit_rows` never runs and a `subscribe` callback parks forever. Subscribing to one view would therefore corrupt the tables the consumer DID describe — strictly worse than the bug being fixed — and it would repeat on every reconnect, since the same frame arrives again. `cache.find` and `schema.new` raise because a CALLER made a mistake at a callsite with a stack; here the server sent something the schema cannot describe, which is the same class of problem as an unknown message tag, and gets the same answer. test_cache.lua had already pinned "unknown table does not raise — the server can send tables the client didn't declare"; that decision stands. Recovering field names from a named readback was considered and rejected. It would make one relation's behaviour depend on which message carried the row — populated on subscribe, then silently stale, because its TransactionUpdate rows stay positional and undecodable. Every row of a view is positional in every message anyway, so for the case #42 is about there is no named half to recover. "No definition" is two causes, kept apart because they point at different mistakes. No entry at all is ordinary and supported: a hand-written schema describes the tables its consumer uses and no others, and the headless conformance run is exactly that — it omits `profile`, `all_types` and the new view, and passes unchanged, PHASE5 included. An entry with no columns is an author slip, so `schema.new` now rejects `columns = {}` at construction, and the cache still reports it for the unvalidated `cache.new(defs)` spelling. `counts.undescribed` is absent when nothing was skipped, so a consumer subscribed only to relations it describes sees a byte-identical `counts` and no extra events. VIEWS ARE NOW EMITTED, because the capture made it small. gen-schema.sh reads `misc_exports[].View`, resolves `return_type`'s `{Array: {Ref: n}}` to that product for the columns, order and types, seeds the typespace from them, and writes the entry into `tables` with `view = true` — so `schema.new(gen.tables)` covers a view with no consumer change. Only the Array form: `spacetimedb.view` also accepts an Option return for a view yielding at most one row, nothing here has seen one on the wire, and it is left undescribed where the guard above reports it rather than described by inference. A view's own primary-key column does not reach the schema endpoint at all, so no `pk` is emitted rather than a guess; `view = true` is what lets tables.lua's `on_update` raise say "view" instead of sending a reader after an annotation that cannot help. PROVEN ON THE WIRE, not argued. `dev/server/index.js` gains a view over `message` projecting it as (text, sender, id) — deliberately reversed, so a decoder reusing the table's order lands a u64 in `text`. Frames 37-42 are a third capture block, appended so 1-36 stay byte-identical, and they settle three things: a view's readback rows are POSITIONAL where a table's are named (`["before subscribe","yuris",1]` in a SubscribeMultiApplied), a view IS incrementally maintained (a TransactionUpdate carries the new projection under the view's name), and an unsubscribe returns its rows as positional deletes. The tests assert against those bytes, on both a schema that describes the view and one that does not. Also: capture-fixtures.mjs now truncates at the Nth IdentityToken so a third block is re-capturable, and `protocol.decode_row`/`decode_typed` document that an empty column list is total loss rather than lenient. The generated schema fixture and session.jsonl were captured from port 4900 (own state dir), not the shared 3000 server, which is why the provenance URL in chat_schema.lua's header moved. Verified: 6 passed, 0 failed; stylua clean; native and HTML5 conformance both PASS including PHASE5. Reverting the guard fails three test files by name ("the skip was reported at all", "an undescribed relation emits its own event", "an undescribed view stores nothing"). 16 mutations of the new logic — each half of the guard, the row accounting, the report shape, the event and its ordering, the schema.new rejection, the view flag and the generated column order — were all caught; two survivors found gaps in the new tests and both gaps were closed.Review — automated reviewer, posted for the record
Review — PR #49
feat/42-views(closes #42)Verdict: closes #42. No blockers. Four should-fixes, three nits. The core fix is correct, verified against live 2.8.2 wire bytes, and well tested.
Suite
6 passed, 0 failed; stylua clean; native conformance PASS (both runs, incl. PHASE5); HTML5 conformance PASS (incl. PHASE5). #39's phase-2 flake did not trigger.1. Should-fixes
S1. The
subscribecallback reports unqualified success while every row is droppedstdb/connection.lua:658(InitialSubscription),:691(subscription_applied) — severity should-fixThe PR argues the skip is "audible without a raise" via
on_event. It is — if you registeredon_event. But the natural way to consume a view issubscribe_query({"SELECT * FROM my_view"}, cb), and that path is silent. Verified:The consumer is told the subscription succeeded, the handle goes active, and every row was discarded.
on_eventdefaults tofunction() end(connection.lua:19) and there is no logging facility anywhere instdb/*.lua— noprint, nosys.*, nothing in cache/connection/schema/tables. So for a non-listener the loss is undiscoverable.This contradicts the repo's own doctrine, stated in
tables.lua:115-118to justifyon_updateraising: silence is indistinguishable from "the server sent nothing yet". Here the SDK chose silence on the one path where the consumer is provably present and holding a callback.Fix (additive, non-breaking): do not touch
callback(err, msg)—connection.lua:739freezes that signature. Instead set the report on the Subscription handle (sub.undescribed) insubscription_applied, wherecountsis already in scope at:691, and document it onon_applied. A view subscriber holds that handle by construction.S2. The generator's view-order resolution has no automated guard
dev/tools/gen-schema.sh:331— severity should-fixThis is the one thing whose failure mode is silent mis-mapping, i.e. worse than the bug being fixed. It is unguarded. Changing line 331 from
($v.return_type.Array.Ref) as $refto(0) as $refregenerates:…the exact corruption #42 exists to prevent — and the suite still passes 6/6. The Lua suite only reads the committed
chat_schema.lua; nothing re-derives it from the tool.Mitigating, and worth crediting: the committed fixture is a faithful witness. I regenerated
chat_schema.luafrom a live server and it reproduced byte-for-byte except line 1's URL comment. Andtest_fixtures.luadoes pin the order — flipping the fixture's view columns to the table's order is CAUGHT.Fix:
conformance.shalready regenerates the schema. Have itdiffthe result against the committed copy (ignoring line 1) and fail on drift. Onediffcloses the only silent-corruption path left in the change.S3. The stated reason for omitting the Option-returning view is now false
dev/tools/gen-schema.sh:160-162and:321-324— severity should-fix (docs, since 1.0 freezes them)Both comments say the Option form is left undescribed because "NOTHING in this repo has ever seen one on the wire". I put one on the wire.
SELECT * FROM item_oneon a live 2.8.2 delivered a row:It is reachable by an ordinary select, exactly like the view in #42's own headline. It is also fully describable: the endpoint reports
return_type.Sum.variants[0].algebraic_type.Ref = 9, which resolves toProduct{only_alpha: String, id: U64}— as resolvable as the Array form, via the samecolumns/coltypeshelpers.The loud path is correct behaviour for it today, and I confirmed it works end to end:
undescribed table=item_one reason="no schema entry" rows=1, nothing cached. So this is not a correctness bug. But the justification is falsified, and it is about to be frozen. Either emit it (resolve thesomevariant'sRef— a one-line jq addition) or restate the reason as a deliberate scope decision rather than an empirical claim that is no longer true.S4. Two undescribed relations in one frame is untested — mutation 7 survived
stdb/cache.lua:346— severity should-fix (test gap only)Mutating
skipped[#skipped + 1] = {...}toskipped[1] = {...}SURVIVED the suite. That collapses every skip report in a frame into one slot, so a consumer subscribed to two undescribed relations hears about only one — breaking the "one notice per skipped relation" promise the README makes andtest_connection.lua:263asserts (but only for a single relation).The behaviour is correct; only the coverage is missing. Verified with a frame carrying
ghost_a,item,ghost_b:Fix: one frame with two distinct undescribed relations, asserting
#counts.undescribed == 2and that entries[1]/[2]name each relation in wire order. That kills the mutation and pins the documented promise.2. Nits
N1.
schema.lua:32-34contradictsschema.new's own validation. The header says a view "has no primary key and cannot have one", butschema.new({v = {columns={"a","b"}, view=true, pk="a"}})is accepted, and the view then gets pk pairing and a workingon_update(verified:events=1 type=update). The behaviour is benign and arguably a useful escape hatch for a hand-written schema whose view projects its table's key — so keep it and fix the comment. The generator never emitspkon a view, so nothing generated is affected.N2.
evt.rowsis a count in an API whererowis a row. Payload is{ type, table, rows, reason }, butcounts.events[].rowis the row itself.evt.rowsreading as "the rows" is a live trap.row_countorskippedavoids it. Frozen if unfixed.N3.
chat_schema.lua:1records an ad-hoc port. Line 1 now sayshttp://127.0.0.1:4900, which appears nowhere else in the repo (gen-schema.shdefaults 3000,dev-server.shusesSTDB_PORT:-3000). Anyone regenerating on the default port dirties the file again. Regenerate against 3000 before merge.3. The skip-and-report decision — reasoning verified, all three limbs
"A raise drops the rows of relations the consumer did describe" — TRUE, and worse than claimed: it is order-dependent. I patched
cache.luato raise and replayed the same frame in both orders:itemrowsghostfirst,itemseconditemfirst,ghostsecondSince the order of
db_update.tablesis the server's choice, a raise would lose described rows nondeterministically. That is a stronger argument than the PR makes."Parks the subscribe callback" — TRUE by construction.
connection.lua:658applies the update before clearingc.pending[msg.request_id]and callingp.callback(nil, msg)at:659-663. A raise at 658 leaves the pending entry forever and never fires the callback. Same shape at:691."No
pcallat any of the four call sites" — TRUE. Sites areconnection.lua:658, 691, 742, 781; the file's onlypcallis:597(settle_one, unrelated).handle_frame(:889) has none, andinit.lua:201callsconn:handle_frame(evt.message)bare inside the extension-websocket callback. A raise would reach Defold's callback and kill the connection.Judgement: right to freeze. But the strongest argument is not the one the PR makes. The PR leans on the event; the real case is that the old behaviour actively corrupted state reachable through the ordinary read API. Running
origin/main's cache against a view frame:So
count()reported a row that had no fields,iter()handed out a table where every access isnil, and a delete of a row the relation never held evicted the row it did hold. The phantom delete is confined to the undescribed relation's own store — a bogus delete against a described pk-less table did not evict, so the PR's "evicts the LAST row of the relation" is precise and does not cross relation boundaries.Is a consumer who never registers
on_eventgenuinely better off? Yes — unambiguously. Before:count()/iter()lied. After: the relation is simply empty, which is honest and inert. Silence plus an accurate absence beats silence plus fieldless garbage that answerscount()with a falsehood. That holds with no handler registered at all. But nothing makes the loss discoverable for that consumer (see S1) — and thesubscribe-callback case is worse than merely undiscoverable, because it affirmatively reports success.The absent-vs-columnless boundary — a hand-written schema can break, but only one already broken
{}rows stored + countedreason = "no schema entry"columnsabsent"schema entry has no columns"columns = {}viaschema.new{}schema.lua:92-100)columns = {}via barecache.new"schema entry has no columns"Omitting tables entirely still works, which is the documented hand-written contract. The only new rejection is
columns = {}— a spelling that could never have worked, since it decoded every row of that relation to a fieldless table. It now fails at construction, loudly, with a message naming the list. Correct call: a hand-written setup that was genuinely working cannot break, because it either named its columns (unaffected) or omitted the table (unaffected).4. The generator's view emission — verified end to end on my own server
Published a probe module on a private 2.8.2 (port 5000, own state dir — the shared 3000 was untouched): table
item (id, alpha, beta)with views deliberately mis-ordered.Column order is correct, and the field names land on the right values. Generated:
Wire: view readback
["b1","a1",1]positional; the table's readback named{"id":1,"alpha":"a1","beta":"b1"}. Decoded throughschema.new(gen.tables)+cache.apply_database_update:The view's own order, not the table's — no mis-mapping. Had the table's order been used,
betawould hold1andidwould hold"b1".item_join→columns = {label, combined, alpha_len, id},types = {string, string, i32, u64}; wire["tag-one","a1/b1",2,1]; decoded correctly. Columns existing in no single table work automatically, because the resolution is of the view's declared product.typesfor a view column: identical vocabulary to a table's. A nested product/sum column also seeds the typespace:item_nested→types = {product_Point, sum_Status, u64}withPoint/Statusemitted. The diff's added seeding line (gen-schema.sh:376-377) is load-bearing — without it those tags would name nothing.Error: name 'item' is used for multiple entities. So$tables + $viewsconcatenating into one Lua constructor (:439) cannot produce a duplicate key that silently overrides a table. Worth knowing the safety is the server's, not the generator's, but it holds.{Sum: {variants:[some,none]}}, not{Option:…}, soselect(.return_type.Array.Ref != null)skips it. See S3.5. The frozen surface
Name
undescribed: good enough to freeze. It mirrors theunhandledprecedent for an unknown message tag — same class of problem, same bare-adjective form, and that parallel is exactly the argument the PR makes. "Undescribed" is also more accurate than "unknown": the server knows the relation fine; this schema does not describe it.Payload
{ type, table, rows, reason }: see N2 onrows. Two smaller points, both permanent:reasonis a human sentence and consumers must string-match it to tell the two causes apart (README documents both literals) — a stable code plus the prose would age better; andtable_idis on the wire but not in the payload, which is the only stable handle if a name is ever reused. Neither is worth blocking on, but both deserve a decision rather than a default.Every
countsconsumer is additive-only — confirmed. With nothing skipped,countskeys are exactlydeletes, events, insertsandundescribed = nil, byte-identical to before. No consumer anywhere iteratespairs(counts), so a new optional key cannot be observed by accident. Full sweep:smoke.script:174,303;test_tables.lua:395-397;test_connection.lua:131;test_fixtures.lua(many);test_cache.lua;README:204,238,501,509.pair_by_keysits inside the newelsebranch (cache.lua:357), reachable only for described relations. Semantics unchanged —test_fixtures.lua:431-439still asserts one update event withinserts=1, deletes=1.store/remove, alsoelse-only. Index structures are built incache.newfromdef.unique; a skipped relation has no def, so there is nothing to maintain and nothing half-maintained.tables.dispatch: takesevt.counts.events, initialized atcache.lua:329and always present — including on the malformed-db_updateearly return at:330-332. All fouremit_rowssites pass a realcounts.count→0,get→nil,iter→0 rows,find→ raisestable "ghost" has no unique column "id".table_def's{}really is safe for lookups, as its new doc comment claims.Should a
view = truedef get adb.tables.<name>facade? Yes — and all six behave sanely.tables.installbuilds one perc.state.schemaentry, so views get one automatically. On a pk-less view:count()iter()find(col, val)has no unique columnunique, sofindis permanently unavailable; the raise is clear and matches pk-less tableson_insert/on_deletedel:b1, ins:b3on_updateview "v" has no primary key…viewwording worksRemoving the facade would be wrong:
count/iter/on_insert/on_deleteare precisely what a view consumer needs and are most of the surface. Theview = trueflag earns its keep in exactly one place — therequire_pkmessage — but that place matters, and mutation 6 confirms it is pinned.6. Fixture integrity — clean
cmpofgit show origin/main:…session.jsonlagainsthead -36of HEAD → exit 0. Numstat6 0: six insertions, zero deletions. The capture tool's truncate-and-reappend hazard did not materialize.IdentityToken,TransactionUpdate(clear_all),TransactionUpdate(send_message, not yet subscribed),SubscribeMultiApplied(message_flip,["before subscribe","yuris",1]),TransactionUpdate(incrementalmessage_flip),UnsubscribeMultiApplied(both rows as deletes).chat_schema.lua: 8 insertions, 1 deletion. The lone deletion is line 1's provenance comment (N3). All four pre-existing tables (all_types,message,player,profile) keep character-identicalcolumns/types/pk/unique/indexes;typespace(Kind,Point,Status) byte-identical. Only real addition is themessage_flipentry. (No new typespace entries — the view'sFlipRowlives inindex.js, not the fixture.)chat_schema.luaanddev/server/index.js;index.jsis +22/−0, a pure addition.updatesentries onmessage(#44's regression evidence) — re-measured with jq.playercase (#35's PHASE5): identity-typedidentity/sponsorpair, score 0 → 42.7. Sanity and the mutation sweep
Suite
6 passed, 0 failed. stylua--check .clean. Native conformance PASS both runs (fresh token, then token-less identity persistence across a process restart), PHASE5 included. HTML5 conformance PASS, PHASE5 included.Spot-checked 12 mutations: 11 caught, 1 survived.
why = nil; restore pre-#42)test_cache.lua,test_connection.lua,test_fixtures.lua. Claim verified precisely.undecodableempty-columns branch unreachabletest_cache.lua)deletestermtest_cache.lua)undescribedemitted after, not before, the frame eventtest_connection.lua)schema.newempty-columns rejection removedtest_cache.lua)require_pkloses theviewwordingtest_tables.lua)reasonstrings made identicalundescribedevent drops the table nametest_connection.lua)test_fixtures.lua)skipped[#skipped+1]→skipped[1]gen-schema.shresolves the wrong typespace refThe author's headline claim — "reverting the guard fails three files by name" — is exact.
8. Does it close #42?
Yes. All five Work items are done: the loud path, the generator emission (verified live), the
view = truemarking, theindex.jsview plus fixture capture, and an explicit, documented decision to store view rows as an append-only list.One acceptance line is met in substance rather than literally, and it is worth stating in the PR body since the issue text will outlive it. #42's Acceptance asks for "a test that an unknown relation raises rather than storing an empty row." Nothing raises. But #42's own Work bullet authorizes the deviation — "should raise, or emit a distinct event" — and the deviation is verifiably better than the raise would have been (§3). Recommend the PR body note that the Acceptance wording was consciously superseded by the Work bullet's latitude, so a future reader does not read the mismatch as an oversight.
Nothing I found causes data loss or silent mis-mapping in the shipped code. S2 is a future risk, not a present defect — the generator is correct today and I proved it on live bytes; it is simply unguarded against regression.