feat(app): support SQL search across sources, and say when one is excluded - #2892
Draft
teeohhem wants to merge 8 commits into
Draft
feat(app): support SQL search across sources, and say when one is excluded#2892teeohhem wants to merge 8 commits into
teeohhem wants to merge 8 commits into
Conversation
`chSqlToAliasMap` walks the parsed SELECT and records each alias against the expression behind it. A NULL literal (`NULL AS "x"`) parses without a source location, so it fell through every branch and was dropped from the map. Callers then treated the alias as a real column: the row-identity WHERE builder, for instance, emitted `x = ...` against a column that does not exist rather than `isNull(NULL)`.
Searching several sources at once needs their rows to share a shape, but each source names its own columns. This builds a SELECT per source that projects that source's semantic expressions under shared aliases — timestamp, service, severity (status for traces), body (span name for traces), duration — and pads anything a source lacks with NULL so every source returns the same columns. Callers can also request extra columns; each resolves to the column where the source has it and NULL where it doesn't. Nothing calls this yet.
Two pure pieces the search page needs before it can span sources. The merge interleaves several already-ordered row streams into one timestamp-ordered list, bounded by a "safe frontier": the timestamp every stream has covered. Rows older than that are held back, so the timeline never shows a gap a slower stream could still fill, and the caller is told which streams are holding the frontier so only those need another page. Streams that error or are excluded stop bounding the frontier rather than freezing the list. The param resolver extends the existing single-source one to a list, deduping, capping, and reporting entries it could not resolve so one bad name doesn't sink the whole selection. Both are unit-tested; nothing calls them yet.
DBSqlRowTable carried the whole denoise flow inline: mine patterns from a sample, find the ones covering more than the noise threshold, then filter the fetched rows against them. Three chained queries and their loading state, in the middle of a component that already does a lot. Pull it into useDenoisedRows plus the small summary that lists what was removed, and have the table call them. Same queries, same keys, same behavior — the component just stops owning it, and a second results table can reuse it rather than copy it. Also threads the last page's row count out of the paginated query, which a caller merging several sources needs to tell "this window is drained" from "this page stopped at its limit".
The search page had one results table bound to one source. It now takes a list: pick up to 3 log/trace sources and their rows interleave into a single timestamp-ordered timeline, each tagged with the source it came from. One source is not a separate path — it is N=1. Its spec carries the user's own SELECT and ORDER BY, so the table renders the authored columns with no source column, sorts, denoises, and reports errors to the page exactly as before. Several sources project the canonical aliases instead, and the client merges the streams behind the safe frontier. There is no UNION: sources can live on different ClickHouse connections, each keeps its own per-table machinery (Lucene serializer, text-index detection, materialized-column rewrites, query settings), and a source that fails degrades to a status chip instead of failing the search. Selections are shareable via ?sources=. Multi-source is Lucene-only; the histogram, total count, and filters sidebar stay single-source until the next change.
The results table already spanned sources; the histogram and the filters sidebar still picked a component per case at the call site. The page now renders SearchHistogram and SearchTotalCount, which take the same per-source list the table does. One source keeps the full DBTimeChart — severity grouping, series drill-down and focus, the pinned tooltip — because a single source has a severity vocabulary to group by. Several sources can't share one, so they stack a count() series each. DBSearchPageFilters takes a list too, so the sidebar people already use works across a selection: facet fields and values merge, value counts are summed so a percentage describes the whole search, "load more" fans out and unions, and pins read as a union and write to every selected source. A source whose table lacks a filtered column is excluded from the results with the reason on its status chip, rather than quietly returning rows that ignore the filter. With one source every path is the one it was before: analysis-mode tabs, denoise, shared filters, and dropping percentages when a distribution query fails.
…luded Multi-source search refused SQL, on the grounds that a SQL WHERE names one table's columns. That reasoning doesn't hold: the hazard it describes — a column name that exists in two sources meaning different things — applies just as much to Lucene, which was allowed. And the case it was really guarding against, a column one source lacks, has a better answer than refusing the language. Both languages now resolve the columns a search references against each source's schema before querying. A source with the columns runs the query; a source without them is excluded and its chip says which column is missing. That also closes a quieter gap: Lucene resolves an unknown field to a condition matching no rows, so `StatusCode:Error` across logs and traces used to drop the log source with nothing on screen to say so. Extraction is deliberately conservative — a query that can't be parsed, or a reference that isn't rooted at a plain column, excludes nobody and falls back to running the query and surfacing any real error per source.
Renders a multi-source search's per-source queries against a mocked metadata layer and asserts the user's condition reaches each one: a SQL condition passes through verbatim to both tables, a Lucene field resolves against each source, and a bare Lucene term lands on each source's own implicit column (Body for logs, SpanName for traces). This is the check the SQL path had been missing — the language used to be refused in multi-source search, so nothing exercised it end to end.
🦋 Changeset detectedLatest commit: c64b8a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR adds multi-source log/trace search, including per-source query execution, schema-aware source exclusion, canonical result projection, merged pagination, source-aware histograms, and SQL/Lucene field extraction.
Confidence Score: 4/5The ordering mismatch should be fixed before merging because valid source configurations can make multi-source pagination return an incomplete or incorrectly advanced timeline. Multi-source merging assumes every source stream is timestamp-descending, but explicit source order expressions are emitted verbatim and can select rows in ascending or unrelated-column order; the oversized new components are an additional non-blocking maintainability concern. Files Needing Attention: packages/app/src/DBSearchPage.tsx, packages/app/src/components/SearchResultsTable.tsx, packages/app/src/components/SearchHistogram.tsx
|
| Filename | Overview |
|---|---|
| packages/app/src/DBSearchPage.tsx | Adds multi-source selection and query planning, but preserves source-defined ordering that can violate the merged pagination contract. |
| packages/app/src/components/SearchResultsTable.tsx | Introduces per-source query streams, frontier-based merging, status chips, and source-aware row details; the new file exceeds the repository size limit. |
| packages/app/src/components/SearchHistogram.tsx | Adds single- and multi-source histogram/count orchestration; the new file exceeds the repository size limit. |
| packages/app/src/utils/multiSourceMerge.ts | Implements conservative timestamp-frontier merging whose correctness depends on every input stream being timestamp-ordered in the declared direction. |
| packages/app/src/hooks/useMultiSourceSearch.ts | Adds schema union, root-column resolution, missing-column detection, and per-source extra-column projection. |
| packages/common-utils/src/core/searchChartConfig.ts | Adds canonical multi-source projections and per-source search-config construction. |
| packages/common-utils/src/queryParser.ts | Adds conservative Lucene AST field extraction while excluding implicit bare terms. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
URL[Search URL and form state] --> Sources[Resolve selected sources]
Sources --> Schema[Load each source schema]
Schema --> Plan[Build per-source query configs]
Plan --> Exclude{Required columns available?}
Exclude -->|No| Status[Show excluded source status]
Exclude -->|Yes| Queries[Run independent ClickHouse queries]
Queries --> Streams[Paginated source streams]
Streams --> Merge[Merge by timestamp frontier]
Merge --> Table[Merged results table]
Plan --> Histograms[Per-source count queries]
Histograms --> Chart[Stacked source histogram]
Reviews (1): Last reviewed commit: "test(common-utils): pin per-source WHERE..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+902
to
+903
| const explicit = isEventSource ? source.orderByExpression?.trim() : undefined; | ||
| if (explicit) return explicit; |
Contributor
There was a problem hiding this comment.
When a selected source defines an ascending or non-timestamp orderByExpression, multiSourceDefaultOrderBy emits it verbatim while pagination and mergeStreams treat every stream as timestamp-descending. The resulting page uses an invalid coverage frontier, causing recent rows to be omitted or the merged timeline to advance incorrectly.
Knowledge Base Used:
| @@ -0,0 +1,618 @@ | |||
| import { useCallback, useEffect, useMemo, useState } from 'react'; | |||
Contributor
There was a problem hiding this comment.
This new 618-line component, together with the 421-line SearchHistogram.tsx, exceeds the repository's 300-line file limit. Splitting query orchestration, stream merging, status rendering, and side-panel behavior into focused modules will reduce the cost and risk of maintaining this feature.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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.
Summary
Stacked on #2886. Removes two limitations from multi-source search, one of them a silent-wrong-answer bug.
SQL search now works across sources. It was refused on the grounds that a SQL
WHEREnames one table's columns. That reasoning doesn't survive scrutiny: the hazard it describes — a column name existing in two sources but meaning different things — applies equally to Lucene, which was allowed. And the case it was actually guarding against, a column one source lacks, has a better answer than refusing the language.A source that can't answer the search now says so. Both languages resolve the columns a search references against each source's schema before querying. Sources that have them run the query; a source that doesn't is excluded with the missing column named on its status chip — the same treatment the sidebar filters already got.
That also closes a quieter gap that shipped earlier in this stack: Lucene resolves an unknown field to a condition matching no rows, so
StatusCode:Erroracross logs and traces silently dropped the log source with nothing on screen to explain it. It now reads "Logs is excluded: this search uses StatusCode, which it doesn't have."Column extraction is deliberately conservative. SQL goes through the existing SQL column extractor (which already ignores function names and keeps map subscripts); Lucene gets a new AST walk that skips bare terms, since those match whichever column a source designates as implicit. A query that can't be parsed, or a reference not rooted at a plain column, excludes nobody and falls back to running the query and surfacing any real per-source error.
Verification
Unit tests cover the extraction and exclusion logic (root-column resolution for plain, backticked, map and JSON references; unparseable queries; unknown column lists). A new rendering test exercises the path that had no coverage at all — it renders the per-source queries through
renderChartConfigand asserts a SQL condition reaches both tables verbatim, a Lucene field resolves against each source, and a bare term lands on each source's own implicit column (Bodyfor logs,SpanNamefor traces).Not yet exercised against live data: the Docker daemon wouldn't start in this environment, so I verified deterministically at the SQL-generation layer instead of clicking through the app. Worth a browser pass before merge.
How to test on Vercel preview
Preview routes: /search
Steps:
ServiceName = 'cart'in the search input and press Enter.StatusCode = 'Unset'and press Enter.