handle more nullable types - #1984
Conversation
… (and other misc type) columns.
🦋 Changeset detectedLatest commit: 82edb64 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.
|
PR Review✅ No critical issues found. The two fixes are correct and follow existing patterns:
|
E2E Test Results✅ All tests passed • 126 passed • 3 skipped • 1043s
Tests ran across 4 shards in parallel. |
|
@copilot can you pls fix lint errors |
|
okay @claude maybe you can help me fix this lint issue... |
|
Claude finished @MikeShi42's task in 57s —— View job Fixed lint issues
|
9ab50bc to
11ee222
Compare
|
|
||
| **Tools handle formatting and linting automatically** via pre-commit hooks. | ||
| Focus on implementation; don't manually format code. | ||
| **After finishing all code edits**, run `yarn lint:fix` to auto-fix formatting |
There was a problem hiding this comment.
after losing the war with agents vs linter, I'm telling agents to always lint - I'm not sure why we did not encourage this anyways? @teeohhem maybe you can lmk
There was a problem hiding this comment.
We do document the pre-commit hook:
Lines 160 to 163 in dea1b66
In theory, the agent should ensure the code is formatted before pushing a new commit.
## Summary Fix ClickHouse query error when expanding log rows with Nullable(DateTime64) columns (and other Nullable types). - The `convertCHDataTypeToJSType` function didn't generically unwrap `Nullable(...)` types, so `Nullable(DateTime64(...))` fell through to the default string comparison instead of using `parseDateTime64BestEffort()` - Added general `Nullable(...)` recursive unwrapping (matching the existing `LowCardinality(...)` pattern) - Hoisted null value handling above the type switch in `processRowToWhereClause` so all column types (Date, Array, Map, etc.) correctly emit `isNull()` for null values ### Screenshots or video N/A — no UI changes. ### How to test locally or on Vercel 1. Set up a ClickHouse table with a `Nullable(DateTime64)` column and ingest some rows (including rows with null values in that column). 2. Open the log explorer and expand a row that has a `Nullable(DateTime64)` column. 3. Verify that clicking into the row no longer returns a 400 error. 4. Verify that clicking into a row where the `Nullable(DateTime64)` column is null correctly filters using `isNull()`. ### References - Related PRs: --- 📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs), [Azure Boards](https://gh.io/cca-azure-boards-docs) or [Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in one click without leaving your project management tool.
## Summary Fix ClickHouse query error when expanding log rows with Nullable(DateTime64) columns (and other Nullable types). - The `convertCHDataTypeToJSType` function didn't generically unwrap `Nullable(...)` types, so `Nullable(DateTime64(...))` fell through to the default string comparison instead of using `parseDateTime64BestEffort()` - Added general `Nullable(...)` recursive unwrapping (matching the existing `LowCardinality(...)` pattern) - Hoisted null value handling above the type switch in `processRowToWhereClause` so all column types (Date, Array, Map, etc.) correctly emit `isNull()` for null values ### Screenshots or video N/A — no UI changes. ### How to test locally or on Vercel 1. Set up a ClickHouse table with a `Nullable(DateTime64)` column and ingest some rows (including rows with null values in that column). 2. Open the log explorer and expand a row that has a `Nullable(DateTime64)` column. 3. Verify that clicking into the row no longer returns a 400 error. 4. Verify that clicking into a row where the `Nullable(DateTime64)` column is null correctly filters using `isNull()`. ### References - Related PRs: --- 📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs), [Azure Boards](https://gh.io/cca-azure-boards-docs) or [Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in one click without leaving your project management tool. Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
Summary
Fix ClickHouse query error when expanding log rows with Nullable(DateTime64) columns (and other Nullable types).
convertCHDataTypeToJSTypefunction didn't generically unwrapNullable(...)types, soNullable(DateTime64(...))fell through to the default string comparison instead of usingparseDateTime64BestEffort()Nullable(...)recursive unwrapping (matching the existingLowCardinality(...)pattern)processRowToWhereClauseso all column types (Date, Array, Map, etc.) correctly emitisNull()for null valuesScreenshots or video
N/A — no UI changes.
How to test locally or on Vercel
Nullable(DateTime64)column and ingest some rows (including rows with null values in that column).Nullable(DateTime64)column.Nullable(DateTime64)column is null correctly filters usingisNull().References
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.