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: 5a32e22

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

This PR prevents unbounded ClickHouse scans during Map-key discovery and propagates source and date-range context through autocomplete consumers.

• Requires bounded ranges for raw-table and text-index Map-key discovery.
• Applies a stable, hour-aligned 24-hour default metadata window.
• Uses path-specific cache keys for raw scans, text indexes, and metadata rollups.
• Wires source and range context through chart, alert, dashboard-filter, and heatmap editors.
• Adds coverage for bounded discovery, cache behavior, window alignment, and API metadata lookup.
• The change since the previous review only clarifies an existing comment about top-N discovery behavior.

Confidence Score: 5/5

The PR appears safe to merge; no outstanding blocking findings remain in the current code.

All previous review threads are resolved, including the explicitly deferred editor-integration coverage, and the latest change only corrects explanatory text without changing behavior. No actionable regression or repository-rule violation was introduced since the previous review.

Important Files Changed

Filename	Overview
packages/common-utils/src/core/metadata.ts	Bounds Map-key discovery, introduces aligned default windows, and separates cache keys by discovery path and scope.
packages/common-utils/src/tests/metadata.test.ts	Adds regression coverage for bounded scans, text-index and rollup windows, and exact-range cache behavior.
packages/app/src/hooks/useMetadata.tsx	Supplies a bounded default range to field metadata requests.
packages/app/src/hooks/useAutoCompleteOptions.tsx	Replaces the static application-time fallback with the shared aligned metadata range.
packages/api/src/controllers/ai.ts	Bounds AI metadata discovery and forwards source timestamp and materialized-view metadata.
packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx	Propagates source and date-range scope to chart autocomplete editors.
packages/app/src/components/DashboardFiltersModal/QueryExpressionFilterEditForm.tsx	Supplies source identity to dashboard-filter autocomplete.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Autocomplete requests field metadata] --> B{Date range available?}
  B -- No --> C[Use aligned 24-hour default]
  B -- Yes --> D[Use caller range]
  C --> E[Widen and align discovery range]
  D --> E
  E --> F{Text index available?}
  F -- Yes --> G[Query overlapping ClickHouse parts]
  F -- No --> H{Metadata rollup available?}
  H -- Yes --> I[Query bounded rollup]
  H -- No --> J{Caller supplied range and timestamp expression?}
  J -- Yes --> K[Run bounded raw-table sample]
  J -- No --> L[Skip Map-key discovery]
  G --> M[Cache and return keys]
  I --> M
  K --> M

Loading

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

[github-actions]

Deep Review

✅ No critical issues found. No P0/P1 defects were introduced by this diff. The change does what it claims: every Map-key discovery path is now bounded (text-index and rollup require an aligned/widened dateRange; the raw scan requires both scanDateRange and timestampValueExpression via canScanTable, otherwise returning [] and warning once per column). The previously-flagged cache-staleness concerns are addressed — defaultFieldMetadataDateRange() aligns via getAlignedDateRange, which rounds the window end up to the next bucket (packages/common-utils/src/core/utils.ts:1081-1085), so the current partial hour is not excluded, and the three-namespace cache scheme (cacheKey / indexCacheKey / rollupCacheKey) keeps scan, index, and rollup answers from colliding.

🟡 P2 — recommended

• packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx:235 — the new sourceId/dateRange propagation across ~9 autocomplete editors (chart, alert, heatmap, dashboard-filter) has no integration or e2e coverage proving each editor resolves its source and surfaces Map-key suggestions; the hook tests mock getAllFields, so broken source/table wiring would pass unnoticed.
  • Fix: Add a focused UI/e2e test exercising the source-only and explicit-range autocomplete paths for the chart, alert, and dashboard-filter editors.
• packages/common-utils/src/core/metadata.ts:691 — partsOverlapFilter dropped its if (!dateRange) return chSql\1`guard and madedateRangerequired, so any caller reaching it with an undefined range now throws ondateRange[0].getTime()instead of degrading;getMapKeysbranches are gated ondateRange &&, but the getKeyValues` text-index caller should be confirmed to always supply one.
  • Fix: Verify the getKeyValues call path always passes a defined dateRange, or restore a guard/early-return there.

🔵 P3 nitpicks (3)

• packages/common-utils/src/core/metadata.ts:719 — getMapKeys juggles five overlapping range variables (rawDateRange, discoveryRange, dateRange, scanDateRange, alignedDateRange) and rebinds dateRange as a new local after destructuring the parameter as rawDateRange, making the shadowed name a readability trap despite the good inline comments.
  • Fix: Rename the derived local (e.g. discoveryDateRange) so it no longer shadows the original parameter name.
• packages/common-utils/src/core/metadata.ts:171 — MetadataCache.seenKeys grows for the lifetime of the long-lived API process (one entry per db.table.column) solely to gate the warn-once log, with no eviction.
  • Fix: Bound or document seenKeys alongside the existing cache, or reset it with the cache.
• packages/app/src/hooks/useMetadata.tsx:252 — the useMultipleAllFields query key uses the raw (possibly undefined) dateRange while the query function fetches with defaultFieldMetadataDateRange(), so the effective 24h window is absent from the key and a fresh hour's window is only picked up on the next refetch rather than on key change.
  • Fix: Include the resolved default window in the query key, or add a comment noting the intra-hour-stability trade-off is intentional.

---

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

Testing gaps: No integration/e2e coverage for the editor sourceId/dateRange autocomplete wiring; cache-key read-back ordering (indexCacheKey then cacheKey) and the empty-scan-result reuse are exercised indirectly but not asserted per-key.

Prior-comment status: The "unrounded now" cache-miss/leak, allowUnboundedScan dead parameter, hand-rolled hour flooring, and "current hour excluded" concerns from earlier review all appear resolved in the current code; the editor-coverage gap (deferred by the author to a follow-up) and the pre-existing abort-signal handling remain the only open items.

[github-actions]

PR Review

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

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

Findings outside the changed lines

1 minor

• 🔵 packages/app/src/components/DashboardFiltersModal/QueryExpressionFilterEditForm.tsx:72 — Filter editor's hand-built TableConnection drops metadataMVs, so the newly-enabled key discovery takes the raw scan → tableConnection is assembled by hand from source.connection/source.from.databaseName/tableName and omits metadataMVs, so getAllFields → getMapKeys skips the rollup branch (metadata.ts:877) and, now that this call site supplies a timestamp expression, falls all the way through to the raw sampledKeys scan — the exact query this PR exists to avoid — on log/trace sources that have a KV rollup MV but no text index. Build it as { ...tcFromSource(source), tableName } so the metric-table override is kept while metadataMVs survives; the same fix removes the cache collision described in the metadata.ts finding.

---

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

[knudtty]

This looks like a good change, I'll review it in depth tomorrow. In the meantime feel free to fix the lint error

[knudtty]

Just one request, the rest looks good

[knudtty]

Let's limit the scope of this PR to just focus on passing through the daterange and sourceids, if we want to handle abort signals like this is should be a separate PR that works for all metadata queries

[niladrix719]

yeah sure

[niladrix719]

Done, abort-signal handling removed, scoped back to dateRange/sourceId

[github-actions]

🔵 minor — Early cache read can serve a raw-scan sample to a rollup-capable caller

cachedKeys = cache.get(indexCacheKey) ?? cache.get(cacheKey) makes the raw-scan entry visible to callers that do have metadataMVs, which the old code never did (it selected exactly one of the metric / MV-aligned / date-suffix keys). Concretely: the dashboard-filter expression editor builds a TableConnection with no metadataMVs (QueryExpressionFilterEditForm.tsx:72) and passes no dateRange, so useMultipleAllFields substitutes defaultFieldMetadataDateRange(); its raw scan writes a sampled key list to ${keyPrefix}.${scoped0}-${scoped1}-${tve}.keys. Open the alert modal for the same source in the same hour (EditAlertModal.tsx:401 uses tcFromSource(source), so MVs are set, same tve, same default window, same maxKeys) and it computes the identical cacheKey, hits it, and returns the unordered sample instead of consulting the rollup's ranked top-N. Gate the ?? cache.get(cacheKey) fallback on !metadataMVs (or include an MV marker in keyPrefix) so the two branches stay disjoint as they were before.