fix: don't scan the whole table to discover Map keys

[niladrix719]

Fixes #3037

Summary

getMapKeys only added a time filter when the caller happened to pass both a date range and a timestamp expression. Several UI autocomplete call sites (chart editor, alert modal, dashboard filters) passed neither. When that happened, it fell back to an unbounded scan across the whole table instead of skipping or narrowing the query

Steps to reproduce

open the chart editor, don't touch the time picker, and start typing a Group By expression on a Map column (ResourceAttributes[). that keystroke sent ClickHouse a query with no time filter, reading every part of the table, not just recent ones

The fix

Both the raw-table scan and the text-index lookup now refuse to run without something to bound them. Field autocomplete falls back to a 24h window when it
knows the timestamp column but not the range, and the call sites that had a source/date range in scope but weren't passing them now do

Before / after

Before:

SELECT token AS key FROM mergeTreeTextIndex('default', 'otel_logs', 'idx_res_attr_key')
WHERE 1
GROUP BY key HAVING key != ''
LIMIT 1000
FORMAT JSON

No WHERE/predicate, full-table scan

After:

SELECT token AS key FROM mergeTreeTextIndex('default', 'otel_logs', 'idx_res_attr_key')
WHERE part_name IN (
  SELECT name FROM system.parts
  WHERE database = 'default' AND table = 'otel_logs' AND active = 1
    AND (min_time >= fromUnixTimestamp64Milli(...) AND min_time <= fromUnixTimestamp64Milli(...))
       OR ...
)
GROUP BY key HAVING key != ''
LIMIT 1000
FORMAT JSON

Known gap

The SQL editors in source-configuration forms still don't pass scope, so Map keys won't autocomplete there. Left as a follow-up, needs the form's in-progress timestampValueExpression, which isn't available the same way

[changeset-bot]

🦋 Changeset detected

Latest commit: d4c52d0

The changes in this PR will be included in the next version bump.

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

[vercel]

@niladrix719 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Greptile Summary

Bounds Map-key discovery so autocomplete cannot trigger unscoped ClickHouse table scans.

• Uses bounded, cached windows for text-index and metadata-rollup discovery.
• Requires a timestamp expression before falling back to a raw table scan.
• Propagates source and date-range context through chart, alert, dashboard-filter, heatmap, and AI metadata paths.
• Adds regression coverage for cache behavior, query bounds, source resolution, and editor wiring.
• The changes since the previous review safely extract the AI metadata regression test into a dedicated file and otherwise clarify comments.

Confidence Score: 5/5

The PR appears safe to merge; no blocking or non-blocking new issue remains after the latest changes.

The latest changes preserve the bounded discovery behavior and safely move the AI metadata regression test into a focused file with sufficient mocks and setup. All previous findings are resolved; niladrix719 explicitly deferred the broader editor-coverage suggestion and accepted the narrow empty-index retry cost, while other fixes or withdrawals addressed the remaining threads.

Important Files Changed

Filename	Overview
packages/common-utils/src/core/metadata.ts	Adds bounded Map-key discovery windows, separate cache scopes, guarded raw scans, and bounded fallback behavior.
packages/app/src/components/SQLEditor/SQLInlineEditor.tsx	Resolves matching source metadata without applying one source’s timestamp or rollup configuration to another table.
packages/app/src/hooks/useMetadata.tsx	Passes optional date-range and timestamp context through shared multi-table field discovery.
packages/api/src/controllers/ai.ts	Supplies AI metadata discovery with a default bounded range, timestamp expression, and metadata rollups.
packages/api/src/controllers/tests/aiMetadata.test.ts	Isolates bounded AI metadata coverage in a focused, fully mocked test file.
packages/app/src/components/SQLEditor/tests/SQLInlineEditor.test.tsx	Covers matching and mismatched source/table identities plus metric-source timestamp behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  E[Autocomplete editor] --> S[Resolve source and table]
  S --> M[Map-key discovery]
  M --> I{Text index available?}
  I -->|Yes| B[Bounded part lookup]
  I -->|No| R{Metadata rollup available?}
  R -->|Yes| W[Bounded rollup query]
  R -->|No| T{Timestamp expression available?}
  T -->|Yes| Q[Bounded raw-table scan]
  T -->|No| X[Skip Map-key discovery]
  B --> C[Cache suggestions]
  W --> C
  Q --> C

Loading

_{Reviews (48): Last reviewed commit: "fix: don't scan the whole table to disco..." | Re-trigger Greptile}

[github-actions]

Deep Review

Intent: bound Map-key metadata discovery in getMapKeys so autocomplete never triggers an unbounded full-table ClickHouse scan — the raw scan now requires a timestampValueExpression, dateRange defaults to a 24h lookback, cache keys are restructured, and UI call sites thread source/date-range scope through. The core refactor is careful and correct in its main paths: I verified every partsOverlapFilter call site now receives a defined dateRange (no crash risk), the SQL stays parameterized (no injection), the cachedKeys fallback chain is internally consistent, and the AI endpoint's default range is hour-aligned so it does not leak a cache entry per request.

✅ No critical (P0/P1) issues found.

🟡 P2 — recommended

• packages/common-utils/src/core/metadata.ts:993 — The text-index branches now always apply partsOverlapFilter with the always-defaulted 24h dateRange, dropping the timestampValueExpression/partition-key guard that streamDistinctIndexValues keeps; on a table whose partition key is not time-derived, system.parts min_time/max_time are unset (0), so the parts filter matches no parts and the index path returns zero Map keys — a regression from the prior partsFilter = 1 behavior when no range was passed.
  • Fix: Gate the text-index partsOverlapFilter on timestampValueExpression presence and partition-key coverage, falling back to chSql\1`asstreamDistinctIndexValues` does.
  • _correctness
• packages/common-utils/src/core/metadata.ts:1006 — The two mergeTreeTextIndex autocomplete queries omit abort_signal: signal while the rollup (:1112) and raw-scan (:1242) queries pass it, so an abandoned autocomplete keeps the text-index query running on ClickHouse and stalls useMultipleAllFields' Promise.allSettled until it finishes.
  • Fix: Pass abort_signal: signal to both text-index query() calls (key branch at :1006 and kv branch at :1042).
  • _{reliability, previous-comments, adversarial}

🔵 P3 nitpicks (8)

• packages/common-utils/src/core/metadata.ts:1259 — The empty-narrow-window widened fallback runs two sequential scans, and because the scan cache key embeds the exact-ms range a live/relative picker can re-run both per tick; it is bounded (≤2 scans, gated on an empty narrow result, still strictly better than the pre-PR unbounded scan).
  • Fix: Cache the widened result under a stable hour-aligned key or short-circuit when the widened range was already scanned this hour.
  • _{performance, adversarial}
• packages/common-utils/src/core/metadata.ts:941 — The raw-scan cacheKey embeds the exact-ms rawDateRange in the never-evicted process-lifetime MetadataCache, so callers passing relative ranges accrete one never-freed entry per distinct window (the hour-aligned default path stays bounded).
  • Fix: Hour-align the scan cache key or add size/TTL eviction to MetadataCache.
  • _{performance, adversarial, reliability, kieran-typescript}
• packages/common-utils/src/core/metadata.ts:1118 — The rollup catch { return [] } runs inside getOrFetch, so a transient rollup failure memoizes [] under rollup.cacheKey and disables the rollup path for the cache lifetime, forcing every later call onto the costlier scan.
  • Fix: Fetch outside getOrFetch and cache only non-empty results, or rethrow so the empty result is not memoized.
  • _reliability
• packages/common-utils/src/core/metadata.ts:1024 — A transient text-index query error returns [] and short-circuits the rollup and raw-scan fallbacks for that call (it self-heals next call and is not cached), so one keystroke yields an empty autocomplete on a blip.
  • Fix: Fall through to the rollup/scan strategies on a caught text-index error instead of return [] (applies to :1024 and :1059).
  • _{reliability, testing}
• packages/common-utils/src/core/metadata.ts:963 — The cachedKeys selection interleaves three cache namespaces and the "MV callers try their rollup first" rule in one nested ??/?: expression, and the local dateRange shadows the caller's parameter concept (raw vs widened vs aligned windows drive different keys).
  • Fix: Extract a named resolveCachedKeys() helper and rename the aligned local to something intent-revealing.
  • _{maintainability, kieran-typescript}
• packages/common-utils/src/core/metadata.ts:319 — widenToLookback/defaultFieldMetadataDateRange and app-side clampCatalogDateRange are documented "twins" that diverge (24h lookback vs 3-day cap) with no shared constant or test guarding the relationship.
  • Fix: Colocate the lookback/cap constants or add a test asserting the intended relationship so the two windows cannot drift silently.
  • _{maintainability}
• packages/common-utils/src/core/metadata.ts:1026 — New/changed behaviors lack tests: the kv text-index branch, the text-index error fall-through, abort_signal forwarding through the text-index branches, and the empty→widened double-scan (including that it does not re-run per distinct ms window); the alert (EditAlertModal) and dashboard-filter (QueryExpressionFilterEditForm) editor wiring is also uncovered while the chart and SQLInlineEditor paths are covered.
  • Fix: Add targeted tests for the kv branch, index error fall-through, abort forwarding, double-scan gating, and the two remaining editor source/date-range paths.
  • _{testing, previous-comments, kieran-typescript, adversarial}
• packages/app/src/components/Sources/SourceForm/TraceTableModelForm.tsx:1 — Source-configuration SQL editors (this file and LogTableModelForm.tsx) do not thread sourceId/dateRange, so scan-path Map columns without a timestampValueExpression now return [] and Map-key autocomplete does not work there; author-acknowledged follow-up, and the optional props give no compile-time signal about which call sites should pass scope.
  • Fix: File a follow-up to thread per-table timestamp/scope into these editors, or document the intended unscoped fallback.
  • _{maintainability, adversarial}

---

Reviewers (9): correctness, adversarial, performance, reliability, testing, maintainability, kieran-typescript, project-standards, previous-comments.

Testing gaps: getMapKeys kv text-index branch and text-index error fall-through are untested; no test asserts abort_signal reaches the text-index queries; the empty→widened double-scan and MetadataCache entry bounding are unverified; alert and dashboard-filter editor source wiring lacks coverage. Most prior-review items were resolved in the current diff (allowUnboundedScan removed, AI range now hour-aligned, no new as any, aiMetadata.test.ts is 74 lines, current-hour keys not hidden); the changeset is present and well-formed. metadata.ts and metadata.test.ts exceed the repo's 300-line guideline but the violations are pre-existing.

[github-actions]

PR Review

3 finding(s): 🔴 0 critical · 🟠 1 major · 🔵 2 minor

1 posted as inline comment(s) on the changed lines. 2 listed below.

Findings outside the changed lines

• 🟠 packages/common-utils/src/core/metadata.ts:993 — Text-index part pruning is now unconditional, with no check that the partition key is time-derived → partsOverlapFilter lost its timestampValueExpression gate, so both getMapKeys text-index branches (lines 993 and 1027) now always emit part_name IN (SELECT ... FROM system.parts WHERE ... min_time/max_time ...). Per this file's own model — partitionKeyCoversTimestamp's doc comment at line 132 ("what decides whether system.parts.min_time is populated") and the guard streamDistinctIndexValues applies at line 851 — min_time/max_time stay at the epoch on a table whose partition key isn't derived from a DateTime column (e.g. PARTITION BY ServiceName, or none). On such a table every part fails all three overlap disjuncts, the index read returns zero rows, and getMapKeys falls through: with no timestampValueExpression it returns [], so ResourceAttributes[ stops autocompleting entirely where before (WHERE 1) it worked; with one, it silently abandons the index and does the full raw scan the PR exists to avoid. Apply the same partitionKeyCoversTimestamp(tableMetadata.partition_key, timestampValueExpression) check the streaming read uses before pruning, and fall back to an unpruned-but-LIMITed index read when it fails, rather than emitting a filter that can match nothing. Related: the new system.parts subquery is also a fresh failure mode for the catch at line 1019, which returns [] outright instead of falling through to the rollup/scan.

1 minor

• 🔵 packages/app/src/components/DBTracePanel.tsx:409 — Trace ID Expression editor passes the wrong sourceId, so the new sameTable guard disables Map-key discovery there → This editor pairs tableConnection={tcFromSource(parentSourceData)} (line 402) with sourceId={sourceId} — the trace source selected in the panel, a different table. The new sameTable check in SQLInlineEditor.tsx:121 correctly detects the mismatch and drops the timestamp expression, which now means getMapKeys skips the raw scan and this editor offers no Map keys at all. parentSourceId is already in scope and guarded non-null at line 397; pass sourceId={parentSourceId} so the editor resolves the source that actually owns the table it introspects, which is also the only way it gets a bounded scan.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering.}