fix(remote): push the source dump instead of the optimizer's own metadata - #199
Merged
Conversation
There was a problem hiding this comment.
Query Doctor Analysis
3 queries analyzed
0 regressed · 0 improved · 0 new · 0 removed
2 pre-existing issues
SELECT "guests"."id", "guests"."session_id", "guests"."username", "guests"."avatar_path", "guests"."color", "guests"."side", "guests"."audio_recording_path", "guests"."audio_recording_public", "gue...
indexassets(event_id, inserted_at desc)
cost 31,003,449 → 1,498 (100% reduction)SELECT * FROM guest_ip_addresses WHERE ip_address = '127.0.0.1';
indexguest_ip_addresses(ip_address)
cost 154,402 → 8 (100% reduction)
Using assumed statistics (10000000 rows/table). For better results, sync production stats.
More detail → get_ci_run({ runId: "019fa561-54a1-75df-ae3c-2b4f4d46739a" }) · view run · docs
…data `applyStatistics` emitted `optimizer.ownMetadata`, which is a dump of the *optimizing* database, not the source. That copy is restored with `--exclude-table-data-and-children` and its statistics are only ever written inside a transaction that rolls back, so every table reports reltuples = -1 and every column stats: null. The relay stores what it receives verbatim, so a push replaces a project's real production statistics with an empty snapshot. Nothing triggered it before: applyStatistics is only reached from updateStatistics, resetStats, and the HTTP import, and none run on their own. Seeding the drift baseline on connect made the drift path reachable, which turns this into a live fault — a drifted project would push the empty dump, baseline itself against reltuples = -1, then see every real table as size drift and re-dump the source every 60 seconds. Push `statsMode.stats` instead, and push nothing for fromAssumption: synthetic defaults are not a project's production statistics. Co-Authored-By: Claude <[email protected]>
veksen
force-pushed
the
fix-push-source-stats
branch
from
July 27, 2026 20:59
63e1cf4 to
ca54187
Compare
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.
Goal
Stop a statistics push from replacing a project's production snapshot with an empty one. Found reviewing #197 after it merged.
What
applyStatisticsemittedoptimizer.ownMetadata. That is a dump of the optimizing database, not the source:QueryOptimizeris constructed againstoptimizingDbUDRL(remote.ts:113), sosetStatisticsdumps from that connection (query-optimizer.ts:156) and passes the result asplannerTables.Statistics.ownMetadataisplannerTables. Core's own comment sayssnapshotcarries the real numbers and passing the same array as both is "rarely what you want".--exclude-table-data-and-children(schema-link.ts:124), andrestoreStatsonly ever runs inside a transaction that rolls back (genalgo.ts:521).So the pushed payload reports
reltuples = -1for every table andstats: nullfor every column.relay.service.tsstores it verbatim, overwriting the single project-level snapshot.Why it matters now
Nothing reached this path before.
applyStatisticsis only called fromupdateStatistics(declared inClientApi, never invoked by the server),resetStats, and the HTTP import.#197 seeds the drift baseline on connect, which is what first makes the drift path reachable in production. A drifted project would then push the empty dump, baseline itself against
reltuples = -1, and on the next poll see every real table as Size Drift — becauseMath.max(-1, 1)gives a denominator of 1, so any table over the 1,000-row floor drifts. The result is a fullDUMP_STATS_SQLagainst the customer's production database every 60 seconds, indefinitely.Not yet live: the deployed analyzer predates #195 and #197. Worth holding the analyzer deploy until this merges.
How
Push
statsMode.stats— the dump we were handed — and baseline against the same array. Push nothing forfromAssumption, since synthetic defaults are not a project's production statistics; that also closes a smaller existing hole whereresetStatspushed the empty dump for any mode, because the emit sat outside the mode check.Tests
apply-statistics.test.tsdrivesapplyStatisticswith a stubbed optimizer whoseownMetadatareportsreltuples = -1, and asserts the emitted payload is the source dump with its real row count. A second case assertsfromAssumptionemits nothing. Both fail against the previous behaviour.