Skip to content

Commit 48a8d32

Browse files
authored
handle more nullable types (#1984)
## 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.
1 parent dea1b66 commit 48a8d32

7 files changed

Lines changed: 78 additions & 7 deletions

File tree

.changeset/metal-radios-pay.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@hyperdx/common-utils": patch
3+
"@hyperdx/app": patch
4+
---
5+
6+
fix: Fixed bug preventing clicking into rows with nullable date types (and other misc type) columns.

AGENTS.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ directory:
5050
- `agent_docs/code_style.md` - Code patterns and best practices (read only when
5151
actively coding)
5252

53-
**Tools handle formatting and linting automatically** via pre-commit hooks.
54-
Focus on implementation; don't manually format code.
53+
**After finishing all code edits**, run `yarn lint:fix` to auto-fix formatting
54+
and lint issues across all packages. Pre-commit hooks handle this when
55+
committing, but if you finish edits without committing, run `yarn lint:fix`
56+
before stopping.
5557

5658
## Key Principles
5759

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"knip": "knip",
4848
"knip:ci": "knip --reporter json",
4949
"lint": "npx nx run-many -t ci:lint",
50+
"lint:fix": "npx nx run-many -t lint:fix",
5051
"version": "make version",
5152
"release": "npx changeset tag && npx changeset publish"
5253
},

packages/app/src/hooks/__tests__/useRowWhere.test.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,25 @@ describe('processRowToWhereClause', () => {
329329
);
330330
});
331331

332+
it('should handle null value on Date column', () => {
333+
const columnMap = new Map([
334+
[
335+
'event_created',
336+
{
337+
name: 'event_created',
338+
type: "Nullable(DateTime64(3, 'UTC'))",
339+
valueExpr: 'event_created',
340+
jsType: JSDataType.Date,
341+
},
342+
],
343+
]);
344+
345+
const row = { event_created: null };
346+
const result = processRowToWhereClause(row, columnMap);
347+
348+
expect(result).toBe('isNull(event_created)');
349+
});
350+
332351
it('should handle null value in default block', () => {
333352
const columnMap = new Map([
334353
[

packages/app/src/hooks/useRowWhere.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ export function processRowToWhereClause(
5454
);
5555
}
5656

57+
// Handle nullish values for all types uniformly
58+
if (value == null) {
59+
return SqlString.format(`isNull(?)`, [SqlString.raw(valueExpr)]);
60+
}
61+
5762
switch (jsType) {
5863
case JSDataType.Date:
5964
return SqlString.format(`?=parseDateTime64BestEffort(?, 9)`, [
@@ -100,10 +105,6 @@ export function processRowToWhereClause(
100105
);
101106

102107
default:
103-
// Handle nullish values
104-
if (value == null) {
105-
return SqlString.format(`isNull(?)`, [SqlString.raw(valueExpr)]);
106-
}
107108
// Handle the case when string is too long
108109
if (value.length > MAX_STRING_LENGTH) {
109110
return SqlString.format(

packages/common-utils/src/clickhouse/__tests__/index.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { extractColumnReferencesFromKey } from '..';
1+
import {
2+
convertCHDataTypeToJSType,
3+
extractColumnReferencesFromKey,
4+
JSDataType,
5+
} from '..';
26

37
describe('extractColumnReferencesFromKey', () => {
48
it('should extract column references from simple column names', () => {
@@ -37,3 +41,39 @@ describe('extractColumnReferencesFromKey', () => {
3741
]);
3842
});
3943
});
44+
45+
describe('convertCHDataTypeToJSType', () => {
46+
it('should handle Nullable(DateTime64) as Date', () => {
47+
expect(convertCHDataTypeToJSType("Nullable(DateTime64(3, 'UTC'))")).toBe(
48+
JSDataType.Date,
49+
);
50+
});
51+
52+
it('should handle Nullable(Int32) as Number', () => {
53+
expect(convertCHDataTypeToJSType('Nullable(Int32)')).toBe(
54+
JSDataType.Number,
55+
);
56+
});
57+
58+
it('should handle Nullable(String) as String', () => {
59+
expect(convertCHDataTypeToJSType('Nullable(String)')).toBe(
60+
JSDataType.String,
61+
);
62+
});
63+
64+
it('should handle LowCardinality(Nullable(String)) as String', () => {
65+
expect(convertCHDataTypeToJSType('LowCardinality(Nullable(String))')).toBe(
66+
JSDataType.String,
67+
);
68+
});
69+
70+
it('should handle DateTime64 as Date', () => {
71+
expect(convertCHDataTypeToJSType("DateTime64(3, 'UTC')")).toBe(
72+
JSDataType.Date,
73+
);
74+
});
75+
76+
it('should handle Nullable(Bool) as Bool', () => {
77+
expect(convertCHDataTypeToJSType('Nullable(Bool)')).toBe(JSDataType.Bool);
78+
});
79+
});

packages/common-utils/src/clickhouse/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ export const convertCHDataTypeToJSType = (
9494
return JSDataType.Dynamic;
9595
} else if (dataType.startsWith('LowCardinality')) {
9696
return convertCHDataTypeToJSType(dataType.slice(15, -1));
97+
} else if (dataType.startsWith('Nullable(')) {
98+
return convertCHDataTypeToJSType(dataType.slice(9, -1));
9799
}
98100

99101
return null;

0 commit comments

Comments
 (0)