Skip to content

Commit ec2ea56

Browse files
Improve Support for Dynamic and JSON(<parameters>) Types (#1251)
1. Improves getJSONColumnNames to support JSON type with arguments 2. Improves useRowWhere to support arrays and JSON types How I tested this: 1. Created a new table and setup various types of formats (`JSON, JSON(max_dynamic_paths = 3, test_key String), Dynamic, Array(Tuple(test_key String, test_value Dynamic)), Map(String, Dynamic)`) 2. Reproduced exact scenario from tickt and fixed (`getJSONColumnNames`) 4. Clicked around the app with various other types, observed that `Dynamic` fields could not be clicked if they had JSON or Array formats. Fixed that (`useRowWhere`) 5. Added tests for changes, ensured existing tests worked for `Dynamuc` useRowWhere by reproducing them in app Fixes HDX-2523
1 parent eaff492 commit ec2ea56

6 files changed

Lines changed: 87 additions & 16 deletions

File tree

‎.changeset/nervous-pots-join.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@hyperdx/app": patch
3+
---
4+
5+
Improve Support for Dynamic and JSON(<parameters>) Types

‎packages/app/src/components/DBRowDataPanel.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,13 @@ export function useRowData({
115115
}
116116

117117
export function getJSONColumnNames(meta: ResponseJSON['meta'] | undefined) {
118-
return meta?.filter(m => m.type === 'JSON').map(m => m.name) ?? [];
118+
return (
119+
meta
120+
// The type could either be just 'JSON' or it could be 'JSON(<parameters>)'
121+
// this is a basic way to match both cases
122+
?.filter(m => m.type === 'JSON' || m.type.startsWith('JSON('))
123+
.map(m => m.name) ?? []
124+
);
119125
}
120126

121127
export function RowDataPanel({

‎packages/app/src/components/DBRowOverviewPanel.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,14 @@ export function RowOverviewPanel({
295295
{...(onPropertyAddClick
296296
? {
297297
onPropertyAddClick,
298-
sqlExpression: `${source.resourceAttributesExpression}['${key}']`,
298+
sqlExpression:
299+
source.resourceAttributesExpression &&
300+
jsonColumns?.includes(
301+
source.resourceAttributesExpression,
302+
)
303+
? // If resource attributes is a JSON column, we need to cast the key to a string so we can run where X in Y queries
304+
`toString(${source.resourceAttributesExpression}.${key})`
305+
: `${source.resourceAttributesExpression}['${key}']`,
299306
}
300307
: {
301308
onPropertyAddClick: undefined,
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { getJSONColumnNames } from '../DBRowDataPanel';
2+
3+
describe('DBRowDataPanel', () => {
4+
describe('getJSONColumnNames', () => {
5+
it('should return JSON column names', () => {
6+
const meta = [
7+
{ name: 'col1', type: 'String' },
8+
{ name: 'col2', type: 'JSON' },
9+
{ name: 'col3', type: 'JSON(1)' },
10+
];
11+
const result = getJSONColumnNames(meta);
12+
expect(result).toEqual(['col2', 'col3']);
13+
});
14+
});
15+
});

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

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,9 @@ describe('processRowToWhereClause', () => {
173173
const row = { dynamic_field: '"quoted_value"' };
174174
const result = processRowToWhereClause(row, columnMap);
175175

176-
expect(result).toBe("toString(dynamic_field)='quoted_value'");
176+
expect(result).toBe(
177+
"toJSONString(dynamic_field) = toJSONString(JSONExtract('\\\"quoted_value\\\"', 'Dynamic'))",
178+
);
177179
});
178180

179181
it('should handle Dynamic columns with escaped values', () => {
@@ -192,7 +194,47 @@ describe('processRowToWhereClause', () => {
192194
const row = { dynamic_field: '{\\"took\\":7, not a valid json' };
193195
const result = processRowToWhereClause(row, columnMap);
194196
expect(result).toBe(
195-
'toString(dynamic_field)=\'{\\"took\\":7, not a valid json\'',
197+
"toJSONString(dynamic_field) = toJSONString(JSONExtract('{\\\\\\\"took\\\\\\\":7, not a valid json', 'Dynamic'))",
198+
);
199+
});
200+
201+
it('should handle Dynamic columns with nested values', () => {
202+
const columnMap = new Map([
203+
[
204+
'dynamic_field',
205+
{
206+
name: 'dynamic_field',
207+
type: 'Dynamic',
208+
valueExpr: 'dynamic_field',
209+
jsType: JSDataType.Dynamic,
210+
},
211+
],
212+
]);
213+
214+
const row = { dynamic_field: "{'foo': {'bar': 'baz'}}" };
215+
const result = processRowToWhereClause(row, columnMap);
216+
expect(result).toBe(
217+
"toJSONString(dynamic_field) = toJSONString(JSONExtract('{\\'foo\\': {\\'bar\\': \\'baz\\'}}', 'Dynamic'))",
218+
);
219+
});
220+
221+
it('should handle Dynamic columns with array values', () => {
222+
const columnMap = new Map([
223+
[
224+
'dynamic_field',
225+
{
226+
name: 'dynamic_field',
227+
type: 'Dynamic',
228+
valueExpr: 'dynamic_field',
229+
jsType: JSDataType.Dynamic,
230+
},
231+
],
232+
]);
233+
234+
const row = { dynamic_field: "['foo', 'bar']" };
235+
const result = processRowToWhereClause(row, columnMap);
236+
expect(result).toBe(
237+
"toJSONString(dynamic_field) = toJSONString(JSONExtract('[\\'foo\\', \\'bar\\']', 'Dynamic'))",
196238
);
197239
});
198240

‎packages/app/src/hooks/useRowWhere.tsx‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,15 @@ export function processRowToWhereClause(
7272
console.warn('Search value/object key too large.');
7373
}
7474
// TODO: update when JSON type have new version
75-
// will not work for array/object dyanmic data
7675

77-
// escaped strings needs raw, becuase sqlString will add another layer of escaping
78-
// data other than array/object will alwayas return with dobule quote(because of CH)
79-
// remove dobule qoute to search correctly
80-
return SqlString.format(`toString(?)='?'`, [
81-
SqlString.raw(valueExpr),
82-
SqlString.raw(
83-
value[0] === '"' && value[value.length - 1] === '"'
84-
? value.slice(1, -1)
85-
: value,
86-
),
87-
]);
76+
// escaped strings needs raw, because sqlString will add another layer of escaping
77+
// data other than array/object will always return with double quote(because of CH)
78+
// remove double quote to search correctly
79+
return SqlString.format(
80+
"toJSONString(?) = toJSONString(JSONExtract(?, 'Dynamic'))",
81+
[SqlString.raw(valueExpr), value],
82+
);
83+
8884
default:
8985
// Handle nullish values
9086
if (value == null) {

0 commit comments

Comments
 (0)