fix(queryBuilder): support JSON column filters and JSON-typed trace tags#1792
Open
fix(queryBuilder): support JSON column filters and JSON-typed trace tags#1792
Conversation
…me bug The E2E fixture now includes a table with a ClickHouse JSON column so the exact SQL our query builder emits for JSON sub-column filters can be validated end-to-end against a real ClickHouse instance. Writing the tests surfaced a real bug: path extraction from a JSON column returns Dynamic, which ClickHouse's `IN` / `NOT IN` reject with ILLEGAL_TYPE_OF_ARGUMENT. Unit tests only asserted the string shape and couldn't catch this. Fix: cast JSON path accesses to `Nullable(String)`. A plain `::String` cast would silently break `IS NULL` (missing keys become empty strings); `Nullable(String)` keeps the null signal intact while satisfying `IN`'s type requirements. Equals / NotEquals / LIKE are unaffected.
adamyeats
force-pushed
the
feat/json-type-filter-support
branch
from
68fc4cf to
745ae4b
Compare
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
isJsonType()type guard so JSON columns can be detected alongside Map/String/etc. rather than falling through to the string catch-all.getFilters(), the column is now cast toNullable(String)andtypeset to'String', soIN/NOT IN/IsEmpty/IsNotEmptyroute through the correct filter-part builders instead of stringifying arrays or skipping the value branch.FilterEditorto the set that actually works on JSON paths (excludesIsEmpty/IsNotEmpty, which are unreliable on JSON sub-columns).traceTags/traceServiceTagsingenerateTraceIdQuery()— previously the generator unconditionally emitted MapmapKeys(...)syntax, which crashes onJSONcolumns (closes Cannot query single trace when using JSON column for ResourceAttributes and ScopeAttributes #1321). Partial fix for JSON type support for Logs and Traces #1424.JSONcolumn +jsonFilter.spec.tscovering Equals / IN / nested path / LIKE).Why
::Nullable(String)and not plain::String?Writing the E2E tests surfaced a runtime bug that the unit tests couldn't have caught: ClickHouse JSON path extraction returns
Dynamic, andIN/NOT INrejectDynamicwithILLEGAL_TYPE_OF_ARGUMENT. The obvious fix is a::Stringcast — but that quietly breaksIS NULL, because::Stringmaps a missing JSON key to an empty string rather than null.::Nullable(String)satisfies both constraints and is a no-op for Equals / NotEquals / LIKE.Scope
fetchPathsForJSONColumns()(still disabled for perf reasons).isStringTypeinutils.ts.StringFilter/MultiFiltershapes since path values are strings.