Conversation
A windowed live query (one given an offset and limit) prepares both live_query_<id>_get and live_query_<id>_get_total_count, but unsubscribe only deallocated the former. The count statement outlived every teardown while its backing view was dropped, so it was left permanently dangling and accumulated for the lifetime of the database.
…down Add regression coverage around the windowed total count deallocation: a non-windowed query must still tear down cleanly without a total count statement, a window of offset 0 / limit 0 is still a window, repeated subscribe/unsubscribe cycles must not accumulate statements, and unsubscribing one windowed query must leave a concurrent one working.
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
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.
Fixes #1111.
Summary
live.query(one given bothoffsetandlimit) prepares two statements,live_query_<id>_get(int, int)andlive_query_<id>_get_total_count, but theunsubscribeteardown only ranDEALLOCATE live_query_<id>_get. The count statement survived every teardown while its backing view was dropped, so it dangled permanently and accumulated for the lifetime of the database (one more per subscribe/unsubscribe cycle of a paginated live query).isWindowedflag so non-windowed queries are untouched (aDEALLOCATEof a never-prepared name would error).packages/pglite/tests/live.test.ts: a windowed round-trip asserting twolive_query_*prepared statements while subscribed and none afterunsubscribe, plus non-windowed and repeated-windowed teardown cases so the guard can't regress either way. Changeset added (patch).The survivor is dangling, not just leaked:
EXECUTE live_query_<id>_get_total_countafter unsubscribe fails withrelation "live_query_<id>_view" does not exist.Decisions
isWindowedis the flag the prepare path already uses to decide whether the count statement exists, so the teardown now deallocates exactly what was prepared. No new state, no new helper.DEALLOCATEunconditionally (errors on non-windowed queries, where the name was never prepared) and notDEALLOCATE ALL(would take out the caller's own prepared statements).How the tests were run:
tests/live.test.tsimports../dist/live/index.js, sodist/live/index.{js,cjs}was rebuilt with esbuild from this branch'ssrc/live/index.ts(same options astsup.config.ts), against the published@electric-sql/pgliteWASM artefacts, on Node 24. Not run here: the fullpnpm build(needs thepostgres-pglitesubmodule and Emscripten) andeslint(needs the workspace install);prettier --checkis clean and the diff adds no types, exports or imports. CI here should cover both.AI assistance: the bug was found and the fix and tests were drafted with AI tooling in my workflow; the test runs above were executed as pasted. I'm responsible for the change and will handle review feedback.