fix(app): serialize JSON column filters with dot access (HDX-5085) - #2864
fix(app): serialize JSON column filters with dot access (HDX-5085)#2864pulpdrew wants to merge 1 commit into
Conversation
Co-authored-by: Drew Davis <pulpdrew@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: fc3afef The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
Greptile SummaryThe PR threads native JSON-column metadata through search-filter and facet serialization so JSON sub-keys use ClickHouse dot access while Map sub-keys retain bracket access.
Confidence Score: 4/5The facet query paths should be gated on successful JSON-column metadata loading before this PR is merged, because they can still emit the invalid SQL this change is intended to eliminate. Facet and Load more queries execute while JSON metadata can still be undefined, causing native JSON sub-keys to fall back to rejected Map bracket syntax; the test-placement issue is independently non-blocking. Files Needing Attention: packages/app/src/components/DBSearchPageFilters/hooks.ts, packages/app/src/components/DBSearchPageFilters.tsx, and the changed DBSearchPageFilters test files
|
| Filename | Overview |
|---|---|
| packages/app/src/components/DBSearchPageFilters/hooks.ts | Makes facet and Load more serialization JSON-aware, but conflates loading or failed metadata with a confirmed empty JSON-column set. |
| packages/app/src/components/DBSearchPageFilters/utils.ts | Adds schema-directed JSON dot-access serialization while preserving existing Map bracket behavior. |
| packages/app/src/searchFilters.tsx | Threads JSON-column metadata through filter-state serialization using stable refs. |
| packages/app/src/DBSearchPage.tsx | Derives JSON columns from active source metadata and supplies them to search-filter state. |
| packages/app/src/components/DBSearchPageFilters.tsx | Supplies JSON metadata to displayed facet serialization, with the same unresolved-metadata fallback used by the hooks. |
| packages/app/src/components/DBSearchPageFilters/jsonAddToFilter.pipeline.test.ts | Adds end-to-end serialization and round-trip coverage but remains outside the required test directory. |
| packages/app/src/components/DBSearchPageFilters/utils.test.ts | Adds focused JSON-versus-Map accessor tests but remains outside the required test directory. |
Sequence Diagram
sequenceDiagram
participant UI as Search filters
participant Meta as Column metadata
participant Serializer as Filter serializer
participant CH as ClickHouse
UI->>Meta: Request JSON column names
Meta-->>UI: Loading / undefined
UI->>Serializer: Serialize facet key with empty JSON set
Serializer-->>UI: ResourceAttributes['region']
UI->>CH: Execute facet query
CH-->>UI: Reject bracket access on JSON
Meta-->>UI: ResourceAttributes is JSON
UI->>Serializer: Re-serialize with JSON set
Serializer-->>UI: ResourceAttributes.`region`
Reviews (1): Last reviewed commit: "fix(app): serialize JSON column filters ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // render as dot access rather than the bracket access ClickHouse rejects on a | ||
| // JSON column. HDX-5085. | ||
| const jsonColumnsSet = useMemo( | ||
| () => new Set(jsonColumns ?? []), |
There was a problem hiding this comment.
When JSON-column metadata is loading or fails, jsonColumns ?? [] treats every column as non-JSON while facet and Load more queries remain enabled. Native JSON sub-keys are therefore serialized with Map bracket access, causing ClickHouse to reject the facet query with an arrayElement-on-JSON error; the initial request shows an error notification, while Load more silently returns no additional values.
|
|
||
| // HDX-5085: filtering a sub-key of a *native* JSON column (e.g. from clicking | ||
| // "Add to Filters" then "Exclude" in the sidebar) must serialize as dot access, | ||
| // not the bracket access ClickHouse rejects on a JSON column with "First |
There was a problem hiding this comment.
The new pipeline tests, along with the added cases in utils.test.ts, are located beside the component implementation rather than in the repository-required __tests__ directory. This conflicts with the prescribed Jest test organization and makes test discovery less consistent for maintainers.
Context Used: AGENTS.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!
E2E Test Results✅ All tests passed • 277 passed • 1 skipped • 1001s
Tests ran across 4 shards in parallel. |
2 participants
Summary
Search page filters on a JSON
ResourceAttributes(or any JSON-typed) column were being serialized with bracket access — e.g.ResourceAttributes['region'] NOT IN ('eu-central-1'). ClickHouse cannot subscript a JSON column, so the query failed with:This reproduced by clicking Add to Filters and then Exclude (or Include) on a value from a JSON column.
Root cause
Filter keys live in a clean, in-memory
FilterState(dot form, e.g.ResourceAttributes.region) and are converted to raw SQL keys at serialization time byescapeFilterStateKeys→toQuotedClickHouseKeyExpression→toClickHouseKeyExpression. That serialization step had no knowledge of column types and unconditionally rewrote every dot-form sub-key to Map bracket form (Col['key']). For a Map column that is correct; for a JSON column it produces SQL ClickHouse rejects.Fix
Thread the set of JSON-typed columns through the serialization path so a sub-key of a JSON column renders as dot access with backtick-quoted segments (
ResourceAttributes.+`region`) instead of bracket access:toClickHouseKeyExpression/toQuotedClickHouseKeyExpressionnow accept an optionaljsonColumnsset; a JSON base column routes throughmergePath's JSON branch. Map columns keep their existing bracket behavior (including the HDX-4369 numeric-sub-key guard).escapeFilterStateKeysanduseSearchPageFilterStateaccept and forwardjsonColumns(via a ref, matching the existingknownColumnspattern).DBSearchPagederives the JSON column set from the active source's columns and passes it in.DBSearchPageFiltersand itshooks.ts) are made JSON-aware too, so facet counts / "Load more" respect an active JSON filter without emitting invalid SQL.The value round-trips: the persisted
ResourceAttributes.+`region`cleans back toResourceAttributes.regioninFilterState, so re-serialization is stable.Tests
utils.test.ts: JSON-aware cases fortoClickHouseKeyExpression/toQuotedClickHouseKeyExpression(dot access, nested paths, fallback to bracket when not JSON / no set provided).jsonAddToFilter.pipeline.test.ts: end-to-end pipeline coverage for the reported HDX-5085 exclude scenario, asserting the exact corrected condition, that the illegalResourceAttributes['bracket subscript is never emitted, valid-SQL checks, and aparseQueryround-trip.All app unit tests for the affected areas pass (146 tests across 4 suites);
yarn lintreports 0 errors andtsc --noEmitis clean.Screenshots or video
N/A — no visual UI changes; this fixes the generated query for JSON-column filters.
How to test on Vercel preview
Preview routes: /search
Steps:
/searchagainst a source whose table has a native JSON column (e.g.ResourceAttributes).ResourceAttributes.region), and click Add to Filters.arrayElement/JSON error, and that the generated SQL usesResourceAttributes.+`region`(dot access) rather thanResourceAttributes['region'].References
Linear Issue: HDX-5085