Add live query kill() and expose the live-query UUID up front#184
Open
emmanuel-keller wants to merge 6 commits into
Open
Add live query kill() and expose the live-query UUID up front#184emmanuel-keller wants to merge 6 commits into
emmanuel-keller wants to merge 6 commits into
Conversation
`selectLive` now starts the live query via the public `LIVE SELECT` query path instead of the `.select(table).live()` builder (whose query id is `pub(crate)`). This lets the driver capture the live-query UUID eagerly from the statement result and expose it via the new `LiveStream.getQueryId()` — available before any notification arrives. Subscription errors (e.g. unknown table) are still surfaced eagerly. Add `Surreal.kill(String)` / `Surreal.kill(UUID)` to terminate a live query by id via a validated `KILL` statement, and enable the previously `@Disabled` `killLiveQuery_byQueryId` test plus new embedded and WebSocket coverage. Note: `kill()` terminates the query (notifications stop) but does not close a client `LiveStream` on either engine — the KILL `Killed` notification carries `session: None` and is dropped by the routers — so `LiveStream.close()` is what releases a local stream. Documented in the `kill` javadoc and asserted by the tests.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
kill() previously only checked the outer query result and discarded the response, so a server-rejected KILL (e.g. an unknown or not-owned live query, which the KILL statement raises as a per-statement error stored at index 0) was swallowed and kill() returned normally. Take/check the index-0 result via take_one_result! so the failure surfaces as a SurrealException.
Collaborator
Author
|
Re-requesting review after addressing the two findings in
Per-comment detail is on the two inline threads. 🤖 Reply by Claude Code |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The surrealdb client SDK builds IndexedResults with `QueryType::Kill => {}`,
discarding the KILL statement result (including any per-statement error), and
its typed kill is `pub(crate)`. There is therefore no public path to observe a
server-side KILL rejection (e.g. a live query owned by another session):
take(0) on the KILL response only ever yields Value::None.
Revert the ineffective take_one_result check added in 54c29e0 (keep
check_query_result! for transport/connection errors) and document kill() as
best-effort in both the javadoc and the code.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The `.select(table).live()` builder unwraps a one-element `Value::Array([Value::Uuid])` result shape (method/live.rs). Mirror that in the raw-query path's UUID extraction so selectLive() preserves the builder's result-shape handling across server/protocol versions. With surrealdb 3.1.4 the `QueryType::Live` path normalizes the id to a bare `Value::Uuid` via `value.into_uuid()` before storing it in IndexedResults, so `take(0)` yields a scalar there (covered by the existing tests); the array arm is defensive parity for any path/version that stores the raw shape.
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Add a Features bullet documenting live queries (selectLive + kill). Remove the two Planned Features entries: "Killing live queries by ID" (implemented in this PR) and "Futures" (no longer a SurrealDB feature).
Both items were removed (one shipped, one no longer a SurrealDB feature), leaving the section empty; drop the heading and the lone feature-request link.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Completes the live-query feature surface:
Surreal.kill(String queryId)/Surreal.kill(java.util.UUID)— terminate a live query by id via a validatedKILLstatement.LiveStream.getQueryId()— the live-query UUID, available immediately (before the first notification).selectLive(table)is reworked internally to start the live query through the publicLIVE SELECTquery path (query("LIVE SELECT * FROM type::table($tb)")) instead of the.select(table).live()builder, whose query id ispub(crate)and therefore unavailable to the driver. The statement result yields the UUID, captured up front and carried on the returnedLiveStream; the notification stream (Response.stream(0)) is driven on the same background-thread machinery as before, so notification delivery, thread-safety, and shutdown are unchanged. Subscription errors (e.g. unknown table) are still surfaced eagerly viatake(0).This also un-disables
LiveQueryTests.killLiveQuery_byQueryId, which was waiting onSurreal.kill(...).Why
Live-query subscription (
selectLive→LiveStream→ notifications) already existed, but there was no way to terminate a live query by id, and the UUID was only reachable after the first notification (viaLiveNotification.getQueryId()).Reviewer notes — kill semantics
kill()terminates the query (notifications stop) but does not close a clientLiveStreamon either engine: theKILL-generatedKillednotification carriessession: None, which both the embedded router and the WebSocket routing drop. A thread blocked inLiveStream.next()keeps waiting after a kill — useLiveStream.close()to release a local stream.kill(id)is for terminating a query you only hold the id of (e.g. read from a notification, or started on another connection). This is documented in thekilljavadoc and asserted by the tests (which check "no notifications after kill", not "stream ends").Tests
LiveQueryTests: enabledkillLiveQuery_byQueryId; addedselectLiveExposesQueryIdUpFront.LiveQueryWebSocketTests: addedselectLive_overWebSocket_kill_stopsNotifications(verified against a local SurrealDB server).cargo clippy,cargo fmt --check, andspotlessCheckall clean.Built against surrealdb 3.1.4.