Skip to content

Commit 1ca6d15

Browse files
committed
feat: Optimize and fix filtering on toStartOfX primary key expressions
1 parent 3332d5e commit 1ca6d15

File tree

8 files changed

+554
-15
lines changed

8 files changed

+554
-15
lines changed

.changeset/fluffy-mails-sparkle.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+
feat: Optimize and fix filtering on toStartOfX primary key expressions

packages/app/src/DBDashboardPage.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import OnboardingModal from './components/OnboardingModal';
6666
import { Tags } from './components/Tags';
6767
import useDashboardFilters from './hooks/useDashboardFilters';
6868
import { useDashboardRefresh } from './hooks/useDashboardRefresh';
69+
import { useOptimizedTimestampValueExpression } from './hooks/useOptimizedTimestampValueExpression';
6970
import { parseAsStringWithNewLines } from './utils/queryParsers';
7071
import api from './api';
7172
import { DEFAULT_CHART_CONFIG } from './ChartUtils';
@@ -158,6 +159,8 @@ const Tile = forwardRef(
158159
const { data: source } = useSource({
159160
id: chart.config.source,
160161
});
162+
const optimizedTimestampValueExpression =
163+
useOptimizedTimestampValueExpression(chart.config.source);
161164

162165
// const prevSource = usePrevious(source);
163166
// const prevChart = usePrevious(chart);
@@ -178,7 +181,9 @@ const Tile = forwardRef(
178181
connection: source.connection,
179182
dateRange,
180183
granularity,
181-
timestampValueExpression: source.timestampValueExpression,
184+
timestampValueExpression:
185+
optimizedTimestampValueExpression ||
186+
source.timestampValueExpression,
182187
from: {
183188
databaseName: source.from?.databaseName || 'default',
184189
tableName: tableName || '',
@@ -189,7 +194,14 @@ const Tile = forwardRef(
189194
});
190195
}
191196
}
192-
}, [source, chart, dateRange, granularity, filters]);
197+
}, [
198+
source,
199+
chart,
200+
dateRange,
201+
granularity,
202+
filters,
203+
optimizedTimestampValueExpression,
204+
]);
193205

194206
const [hovered, setHovered] = useState(false);
195207

packages/app/src/DBSearchPage.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ import PatternTable from './components/PatternTable';
100100
import { DBSearchHeatmapChart } from './components/Search/DBSearchHeatmapChart';
101101
import SourceSchemaPreview from './components/SourceSchemaPreview';
102102
import { useTableMetadata } from './hooks/useMetadata';
103+
import { useOptimizedTimestampValueExpression } from './hooks/useOptimizedTimestampValueExpression';
103104
import { useSqlSuggestions } from './hooks/useSqlSuggestions';
104105
import {
105106
parseAsSortingStateString,
@@ -488,6 +489,8 @@ function useSearchedConfigToChartConfig({
488489
id: source,
489490
});
490491
const defaultOrderBy = useDefaultOrderBy(source);
492+
const optimizedTimestampValueExpression =
493+
useOptimizedTimestampValueExpression(source);
491494

492495
return useMemo(() => {
493496
if (sourceObj != null) {
@@ -509,7 +512,9 @@ function useSearchedConfigToChartConfig({
509512
...(filters != null ? { filters } : {}),
510513
where: where ?? '',
511514
whereLanguage: whereLanguage ?? 'sql',
512-
timestampValueExpression: sourceObj.timestampValueExpression,
515+
timestampValueExpression:
516+
optimizedTimestampValueExpression ||
517+
sourceObj.timestampValueExpression,
513518
implicitColumnExpression: sourceObj.implicitColumnExpression,
514519
connection: sourceObj.connection,
515520
displayType: DisplayType.Search,
@@ -527,6 +532,7 @@ function useSearchedConfigToChartConfig({
527532
where,
528533
whereLanguage,
529534
defaultOrderBy,
535+
optimizedTimestampValueExpression,
530536
]);
531537
}
532538

packages/app/src/components/DBEditTimeChartForm.tsx

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import { TimePicker } from '@/components/TimePicker';
5757
import { IS_LOCAL_MODE } from '@/config';
5858
import { GranularityPickerControlled } from '@/GranularityPicker';
5959
import { useFetchMetricResourceAttrs } from '@/hooks/useFetchMetricResourceAttrs';
60+
import { useOptimizedTimestampValueExpression } from '@/hooks/useOptimizedTimestampValueExpression';
6061
import SearchInputV2 from '@/SearchInputV2';
6162
import { getFirstTimestampValueExpression, useSource } from '@/source';
6263
import { parseTimeQuery } from '@/timeQuery';
@@ -490,6 +491,9 @@ export default function EditTimeChartForm({
490491
ChartConfigWithDateRange | undefined
491492
>(undefined);
492493

494+
const optimizedTimestampValueExpression =
495+
useOptimizedTimestampValueExpression(tableSource?.id);
496+
493497
const onSubmit = useCallback(() => {
494498
handleSubmit(form => {
495499
setChartConfig(form);
@@ -498,7 +502,9 @@ export default function EditTimeChartForm({
498502
const newConfig = {
499503
...form,
500504
from: tableSource.from,
501-
timestampValueExpression: tableSource.timestampValueExpression,
505+
timestampValueExpression:
506+
optimizedTimestampValueExpression ||
507+
tableSource.timestampValueExpression,
502508
dateRange,
503509
connection: tableSource.connection,
504510
implicitColumnExpression: tableSource.implicitColumnExpression,
@@ -516,7 +522,14 @@ export default function EditTimeChartForm({
516522
);
517523
}
518524
})();
519-
}, [handleSubmit, setChartConfig, setQueriedConfig, tableSource, dateRange]);
525+
}, [
526+
handleSubmit,
527+
setChartConfig,
528+
setQueriedConfig,
529+
tableSource,
530+
dateRange,
531+
optimizedTimestampValueExpression,
532+
]);
520533

521534
useEffect(() => {
522535
if (submitRef) {
@@ -589,7 +602,9 @@ export default function EditTimeChartForm({
589602
},
590603
],
591604
dateRange,
592-
timestampValueExpression: tableSource.timestampValueExpression,
605+
timestampValueExpression:
606+
optimizedTimestampValueExpression ||
607+
tableSource.timestampValueExpression,
593608
connection: tableSource.connection,
594609
from: tableSource.from,
595610
limit: { limit: 200 },
@@ -600,7 +615,13 @@ export default function EditTimeChartForm({
600615
granularity: undefined,
601616
}
602617
: null,
603-
[queriedConfig, tableSource, dateRange, queryReady],
618+
[
619+
queriedConfig,
620+
tableSource,
621+
dateRange,
622+
queryReady,
623+
optimizedTimestampValueExpression,
624+
],
604625
);
605626

606627
// Need to force a rerender on change as the modal will not be mounted when initially rendered
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
import { renderHook } from '@testing-library/react';
2+
3+
import * as sourceModule from '@/source';
4+
5+
import * as metadataModule from '../useMetadata';
6+
import {
7+
optimizeTimestampValueExpression,
8+
useOptimizedTimestampValueExpression,
9+
} from '../useOptimizedTimestampValueExpression';
10+
11+
// Mock the dependencies
12+
jest.mock('@/layout', () => ({
13+
withAppNav: (component: any) => component,
14+
}));
15+
16+
describe('useOptimizedTimestampValueExpression', () => {
17+
beforeEach(() => {
18+
jest.clearAllMocks();
19+
});
20+
21+
describe('optimizeTimestampValueExpression', () => {
22+
const testCases = [
23+
{
24+
timestampValueExpression: 'Timestamp',
25+
primaryKey: 'Timestamp',
26+
expected: 'Timestamp',
27+
},
28+
{
29+
timestampValueExpression: 'Timestamp',
30+
primaryKey: undefined,
31+
expected: 'Timestamp',
32+
},
33+
{
34+
timestampValueExpression: 'Timestamp',
35+
primaryKey: '',
36+
expected: 'Timestamp',
37+
},
38+
{
39+
// Traces Table
40+
timestampValueExpression: 'Timestamp',
41+
primaryKey: 'ServiceName, SpanName, toDateTime(Timestamp)',
42+
expected: 'Timestamp',
43+
},
44+
{
45+
// Optimized Traces Table
46+
timestampValueExpression: 'Timestamp',
47+
primaryKey:
48+
'toStartOfHour(Timestamp), ServiceName, SpanName, toDateTime(Timestamp)',
49+
expected: 'Timestamp, toStartOfHour(Timestamp)',
50+
},
51+
{
52+
// Unsupported for now as it's not a great primary key, want to just
53+
// use default behavior for this
54+
timestampValueExpression: 'Timestamp',
55+
primaryKey: 'toDateTime(Timestamp), ServiceName, SpanName, Timestamp',
56+
expected: 'Timestamp',
57+
},
58+
{
59+
// Inverted primary key order, we should not try to optimize this
60+
timestampValueExpression: 'Timestamp',
61+
primaryKey:
62+
'ServiceName, toDateTime(Timestamp), SeverityText, toStartOfHour(Timestamp)',
63+
expected: 'Timestamp',
64+
},
65+
{
66+
timestampValueExpression: 'Timestamp',
67+
primaryKey: 'toStartOfHour(Timestamp), other_column, Timestamp',
68+
expected: 'Timestamp, toStartOfHour(Timestamp)',
69+
},
70+
{
71+
// When the user has already manually configured an optimized timestamp value expression
72+
timestampValueExpression: ' toStartOfHour(Timestamp), Timestamp',
73+
primaryKey: 'toStartOfHour(Timestamp), other_column, Timestamp',
74+
expected: ' toStartOfHour(Timestamp), Timestamp',
75+
},
76+
{
77+
timestampValueExpression: 'Timestamp',
78+
primaryKey:
79+
'toStartOfInterval(Timestamp, INTERVAL 1 HOUR), other_column, Timestamp',
80+
expected: 'Timestamp, toStartOfInterval(Timestamp, INTERVAL 1 HOUR)',
81+
},
82+
{
83+
// test variation of toUnixTimestamp
84+
timestampValueExpression: 'Timestamp',
85+
primaryKey:
86+
'toStartOfMinute(Timestamp), user_id, status, toUnixTimestamp64Nano(Timestamp)',
87+
expected: 'Timestamp, toStartOfMinute(Timestamp)',
88+
},
89+
{
90+
// TODO TimestampTime is not matched since it is not in the timestampValueExpression
91+
timestampValueExpression: 'Timestamp',
92+
primaryKey:
93+
'toStartOfMinute(TimestampTime), user_id, status, Timestamp',
94+
expected: 'Timestamp',
95+
},
96+
] as const;
97+
98+
it.each(testCases)(
99+
'should return optimized expression $expected for original expression $timestampValueExpression and primary key $primaryKey',
100+
({ timestampValueExpression, primaryKey, expected }) => {
101+
const actual = optimizeTimestampValueExpression(
102+
timestampValueExpression,
103+
primaryKey,
104+
);
105+
106+
expect(actual).toBe(expected);
107+
},
108+
);
109+
});
110+
111+
describe('useOptimizedTimestampValueExpression', () => {
112+
it('should handle null source ungracefully', () => {
113+
jest.spyOn(sourceModule, 'useSource').mockReturnValue({
114+
data: null,
115+
isLoading: false,
116+
error: null,
117+
} as any);
118+
119+
jest.spyOn(metadataModule, 'useTableMetadata').mockReturnValue({
120+
data: null,
121+
isLoading: false,
122+
error: null,
123+
} as any);
124+
125+
const { result } = renderHook(() =>
126+
useOptimizedTimestampValueExpression(null),
127+
);
128+
129+
expect(result.current).toBe(undefined);
130+
});
131+
132+
it('should handle undefined sourceID ungracefully', () => {
133+
jest.spyOn(sourceModule, 'useSource').mockReturnValue({
134+
data: null,
135+
isLoading: false,
136+
error: null,
137+
} as any);
138+
139+
jest.spyOn(metadataModule, 'useTableMetadata').mockReturnValue({
140+
data: null,
141+
isLoading: false,
142+
error: null,
143+
} as any);
144+
145+
const { result } = renderHook(() =>
146+
useOptimizedTimestampValueExpression(undefined),
147+
);
148+
149+
expect(result.current).toBe(undefined);
150+
});
151+
152+
it('should handle complex Timestamp expressions', () => {
153+
const mockSource = {
154+
timestampValueExpression: 'toDateTime(timestamp_ms / 1000)',
155+
};
156+
157+
const mockTableMetadata = {
158+
primary_key:
159+
'toStartOfHour(toDateTime(timestamp_ms / 1000)), toDateTime(timestamp_ms / 1000)',
160+
};
161+
162+
jest.spyOn(sourceModule, 'useSource').mockReturnValue({
163+
data: mockSource,
164+
isLoading: false,
165+
error: null,
166+
} as any);
167+
168+
jest.spyOn(metadataModule, 'useTableMetadata').mockReturnValue({
169+
data: mockTableMetadata,
170+
isLoading: false,
171+
error: null,
172+
} as any);
173+
174+
const { result } = renderHook(() =>
175+
useOptimizedTimestampValueExpression('source-id'),
176+
);
177+
178+
expect(result.current).toBe(
179+
'toDateTime(timestamp_ms / 1000), toStartOfHour(toDateTime(timestamp_ms / 1000))',
180+
);
181+
});
182+
183+
it('should memoize result correctly when dependencies change', () => {
184+
const mockSource1 = {
185+
timestampValueExpression: 'timestamp1',
186+
};
187+
188+
const mockSource2 = {
189+
timestampValueExpression: 'timestamp2',
190+
};
191+
192+
const useSourceSpy = jest
193+
.spyOn(sourceModule, 'useSource')
194+
.mockReturnValue({
195+
data: mockSource1,
196+
isLoading: false,
197+
error: null,
198+
} as any);
199+
200+
jest.spyOn(metadataModule, 'useTableMetadata').mockReturnValue({
201+
data: undefined,
202+
isLoading: false,
203+
error: null,
204+
} as any);
205+
206+
const { result, rerender } = renderHook(() =>
207+
useOptimizedTimestampValueExpression('source-id'),
208+
);
209+
210+
expect(result.current).toBe('timestamp1');
211+
212+
// Update the mock to return different data
213+
useSourceSpy.mockReturnValue({
214+
data: mockSource2,
215+
isLoading: false,
216+
error: null,
217+
} as any);
218+
219+
rerender();
220+
221+
expect(result.current).toBe('timestamp2');
222+
});
223+
});
224+
});

0 commit comments

Comments
 (0)