feat(query-engine): migrate the last two handlers (58/58) - #347
Merged
Conversation
cloudflareInfraZoneBreakdown and serviceOperations, the two that were left because their control flow is not query construction. ZoneBreakdown runs three queries in parallel and then a fourth whose grouping keys come from the first's ranked output. `topKeys` therefore rides in the PAYLOAD rather than being derived inside compile -- a def has no way to see a previous query's result. The caller still skips the fourth entirely when the key list is empty. Its coverage sub-query stays deliberately unfiltered: it answers "what did the poller collect here", which is how the UI distinguishes "not collected yet" from "no traffic". serviceOperations has a rollup form and a raw form of each of its two queries. The CHOICE stays in the handler because it is policy: a feature flag selects the rollup, and a typed isMissingServiceOperationsRollup error falls back to raw at runtime, flipping a flag the timeseries query then honors too. The defs own compile, profile and context; the handler owns the fallback. Rollup/raw pairs share an id, matching the context their spans already report -- the fallback is recorded separately as query.rollup.fallback. Also removes dead code the earlier passes left behind: an unused `params` object in each of the two handlers above, and a whole conditional CH.compile in planetscaleInfraTimeseries that its def had already superseded. query-engine.http.ts now contains ZERO CH.compile calls and is down from 3275 to 1774 lines. The three remaining handlers -- execute, executeQueryBuilder and executeRawSql -- are the QuerySpec and raw-SQL surfaces and should not become QueryDefs. Verified: both packages typecheck. Test suites not run locally by request; CI covers them, including the SQL baseline and the ClickHouse DESCRIBE sweep.
🍁 Maple PR previewNote Preview resources were removed when this pull request closed. Final commit |
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.
Stacked on #346 — review that first.
Completes the handler migration.
query-engine.http.tsnow contains zeroCH.compilecalls and is down from 3275 to 1774 lines (−46%).The two that were left
Both were left because their complexity is control flow, not query construction.
cloudflareInfraZoneBreakdownThree queries in parallel, then a fourth whose grouping keys come from the first's ranked output.
topKeysrides in the payload rather than being derived insidecompile— a def has no way to see a previous query's result:The caller still skips it entirely when the key list is empty. The coverage sub-query stays deliberately unfiltered — it answers "what did the poller collect here", which is how the UI distinguishes "not collected yet" from "no traffic" for a window predating the dataset.
serviceOperationsA rollup form and a raw form of each of its two queries. The choice stays in the handler because it's policy: a feature flag selects the rollup, and a typed
isMissingServiceOperationsRolluperror falls back to raw at runtime — flipping a flag that the timeseries query then honors too.The defs own compile/profile/context; the handler owns the fallback. Rollup/raw pairs share an id, matching the context their spans already report; the fallback is recorded separately as
query.rollup.fallback.This handler is also why the
withDepsremoval in #346 mattered —runQuerycomposes inside itscachedDirectonly because the curried runners carryR = never.Dead code removed
Earlier passes left behind an unused
paramsobject in each of the two handlers above, plus a whole conditionalCH.compileinplanetscaleInfraTimeseriesthat its def had already superseded — it was still compiling a query nobody executed.Not migrated, by design
execute,executeQueryBuilder,executeRawSql— the QuerySpec and raw-SQL surfaces. Forcing them intoQueryDefwould be a semantics rewrite, not a refactor.Testing
🤖 Generated with Claude Code