Support subquery moves across AND/OR/NOT WHERE clauses#4051
Support subquery moves across AND/OR/NOT WHERE clauses#4051
Conversation
c7da8bb to
f33c781
Compare
6669eb8 to
a1e0fc1
Compare
226f9b3 to
203f4fc
Compare
67f228e to
438e6cf
Compare
66d52fe to
fb8a7d9
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4051 +/- ##
==========================================
- Coverage 89.20% 86.29% -2.92%
==========================================
Files 25 51 +26
Lines 2520 3553 +1033
Branches 633 638 +5
==========================================
+ Hits 2248 3066 +818
- Misses 270 485 +215
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
413acaf to
4208d42
Compare
Claude Code ReviewSummaryThis PR is a substantial architectural overhaul that adds DNF-based WHERE clause decomposition to enable subquery move-ins/outs across What's Working Well
Issues FoundImportant (Should Fix)1. File: def handle_info({:query_move_in_error, error, stacktrace}, state) do
Logger.error(...)
reraise(error, stacktrace)
# No-op as the raise will crash the process ← the comment admits the problem
stop_and_clean(state) ← unreachable
end
Suggested fix — use a controlled stop rather than a crash: def handle_info({:query_move_in_error, error, stacktrace}, state) do
Logger.error("Error querying move in for #{state.shape_handle}: #{Exception.format(:error, error, stacktrace)}")
{:stop, {error, stacktrace}, state}
endThis still calls 2. Mixed-polarity subquery validation happens at runtime, not at shape-creation time File:
A shape with mixed polarity would be created successfully and written to storage, then fail the first time it tries to serve data. Depending on how callers handle Suggested fix: call 3. File: defp splice(%{active_move: active_move} = state) do
with {:ok, splice_plan} <- SplicePlan.build(active_move, state.shape_info) do
...
end
endIf 4. No linked issue The PR description is detailed, but it does not reference any issue number. PRs should link to the issue they address so context and requirements are discoverable from the code history. Suggestions (Nice to Have)5. File: dep_index = subquery_ref |> List.last() |> String.to_integer()
6. The plan is deterministic for a given shape and involves AST traversal + SQL generation for all positions. For shapes with complex subquery DNFs, repeated compilation per transaction adds up. Consider storing the compiled plan in the consumer's process state (alongside 7. Missing telemetry for buffer overflow and splice completion
8. Changeset description could be more specific about protocol fields The current changeset note says "changes the wire protocol" but doesn't describe what changed (new Issue ConformanceNo linked issue is present, so there are no formal requirements to cross-reference. The PR description is thorough and accurately describes the architecture. The compatibility matrix ( Review iteration: 1 | 2026-04-22 |
Summary
This PR lets shapes with boolean combinations around subqueries stay live when dependency rows move. Previously, shapes that used
AND,OR, orNOTaround a subquery would return409on a move, forcing the client to throw away the shape and resync it from scratch. For large shapes that could mean a very expensive full resync; this PR avoids that by reconciling those changes in-stream.The core of the PR is a rewrite of the subquery foundation. Move-ins are now handled as exact log splices: buffer outer-table transactions, run the move-in query, and insert its results into the shape log at the precise point where they become valid. That gives subqueries a much more reliable ordering model, removes a large class of move-handling edge cases, and gives us a stable base for richer boolean
WHEREsupport over subqueries.Compatibility
This PR changes the wire protocol.
The sync service from this branch is not compatible with
elixir-clientfrom before this PR. For the TanStack DB client, this requires:@tanstack/db >= 0.6.2@tanstack/electric-db-collection >= 0.3.0The protocol change is needed because DNF-based subquery shapes need more than tags alone. Rows can now stay in the shape for multiple independent reasons, so the server sends real
active_conditionsvalues and position-awaremove-inbroadcast control messages. That lets clients update the truth value of the affected DNF positions for rows they already have locally, re-evaluate inclusion correctly, and avoid unnecessary deletes or full resyncs.Architecture
WHEREclause into a DNF plan and use that plan consistently for routing, move planning, SQL generation, tags, andactive_conditionsSubqueryIndexthat each shape consumer seeds and updates from its own dependency views, keeping the reverse index exactly in sync with consumer stateElectric.Shapes.Consumeraround explicit event handlers and ordered effects, with a single move queue and explicit buffering/splice handlingSecondary changes
tags,active_conditions, and move broadcasts