Skip to content

Commit 943ddb8

Browse files
pulpdrewpulpdrew
committed
feat: Separate useChunkedQueriedChartConfig from useQueriedChartConfig
1 parent 3e73289 commit 943ddb8

File tree

3 files changed

+134
-108
lines changed

3 files changed

+134
-108
lines changed

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

Lines changed: 85 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { useClickhouseClient } from '@/clickhouse';
99

1010
import {
1111
getGranularityAlignedTimeWindows,
12-
useQueriedChartConfig,
12+
useChunkedQueriedChartConfig,
1313
} from '../useChartConfig';
1414

1515
// Mock the clickhouse module
@@ -97,6 +97,48 @@ describe('useChartConfig', () => {
9797
).toEqual([undefined]);
9898
});
9999

100+
it('returns windows aligned to the granularity if the granularity is auto', () => {
101+
expect(
102+
getGranularityAlignedTimeWindows(
103+
{
104+
dateRange: [
105+
new Date('2023-01-10 00:00:00'),
106+
new Date('2023-01-10 01:00:00'),
107+
],
108+
granularity: 'auto', // will be 1 minute
109+
timestampValueExpression: 'TimestampTime',
110+
} as ChartConfigWithOptDateRange,
111+
[
112+
30, // 30s
113+
5 * 60, // 5m
114+
60 * 60, // 1hr
115+
],
116+
),
117+
).toEqual([
118+
{
119+
dateRange: [
120+
new Date('2023-01-10 00:59:00'), // Aligned to minute, the auto-inferred granularity
121+
new Date('2023-01-10 01:00:00'),
122+
],
123+
dateRangeEndInclusive: undefined,
124+
},
125+
{
126+
dateRange: [
127+
new Date('2023-01-10 00:54:00'),
128+
new Date('2023-01-10 00:59:00'),
129+
],
130+
dateRangeEndInclusive: false,
131+
},
132+
{
133+
dateRange: [
134+
new Date('2023-01-10 00:00:00'),
135+
new Date('2023-01-10 00:54:00'),
136+
],
137+
dateRangeEndInclusive: false,
138+
},
139+
]);
140+
});
141+
100142
it('returns windows aligned to the granularity if the granularity is larger than the window size', () => {
101143
expect(
102144
getGranularityAlignedTimeWindows(
@@ -269,7 +311,7 @@ describe('useChartConfig', () => {
269311
});
270312
});
271313

272-
describe('useQueriedChartConfig', () => {
314+
describe('useChunkedQueriedChartConfig', () => {
273315
let queryClient: QueryClient;
274316
let wrapper: React.ComponentType<{ children: any }>;
275317
let mockClickhouseClient: jest.Mocked<ClickhouseClient>;
@@ -319,9 +361,12 @@ describe('useChartConfig', () => {
319361

320362
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
321363

322-
const { result } = renderHook(() => useQueriedChartConfig(config), {
323-
wrapper,
324-
});
364+
const { result } = renderHook(
365+
() => useChunkedQueriedChartConfig(config),
366+
{
367+
wrapper,
368+
},
369+
);
325370

326371
await waitFor(() => expect(result.current.isSuccess).toBe(true));
327372
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -364,9 +409,12 @@ describe('useChartConfig', () => {
364409

365410
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
366411

367-
const { result } = renderHook(() => useQueriedChartConfig(config), {
368-
wrapper,
369-
});
412+
const { result } = renderHook(
413+
() => useChunkedQueriedChartConfig(config),
414+
{
415+
wrapper,
416+
},
417+
);
370418

371419
await waitFor(() => expect(result.current.isSuccess).toBe(true));
372420
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -413,55 +461,8 @@ describe('useChartConfig', () => {
413461

414462
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
415463

416-
const { result } = renderHook(() => useQueriedChartConfig(config), {
417-
wrapper,
418-
});
419-
420-
await waitFor(() => expect(result.current.isSuccess).toBe(true));
421-
await waitFor(() => expect(result.current.isFetching).toBe(false));
422-
423-
// Should only be called once since chunking is disabled without timestampValueExpression
424-
expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledTimes(1);
425-
expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({
426-
config,
427-
metadata: expect.any(Object),
428-
opts: {
429-
abort_signal: expect.any(AbortSignal),
430-
},
431-
});
432-
expect(result.current.data).toEqual({
433-
data: mockResponse.data,
434-
meta: mockResponse.meta,
435-
rows: mockResponse.rows,
436-
});
437-
});
438-
439-
it('fetches data without chunking when disableQueryChunking is true', async () => {
440-
const config = createMockChartConfig({
441-
dateRange: [
442-
new Date('2025-10-01 00:00:00Z'),
443-
new Date('2025-10-02 00:00:00Z'),
444-
],
445-
granularity: '1 hour',
446-
});
447-
448-
const mockResponse = createMockQueryResponse([
449-
{
450-
'count()': '71',
451-
SeverityText: 'info',
452-
__hdx_time_bucket: '2025-10-01T00:00:00Z',
453-
},
454-
{
455-
'count()': '73',
456-
SeverityText: 'info',
457-
__hdx_time_bucket: '2025-10-02T00:00:00Z',
458-
},
459-
]);
460-
461-
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
462-
463464
const { result } = renderHook(
464-
() => useQueriedChartConfig(config, { disableQueryChunking: true }),
465+
() => useChunkedQueriedChartConfig(config),
465466
{
466467
wrapper,
467468
},
@@ -470,7 +471,7 @@ describe('useChartConfig', () => {
470471
await waitFor(() => expect(result.current.isSuccess).toBe(true));
471472
await waitFor(() => expect(result.current.isFetching).toBe(false));
472473

473-
// Should only be called once since chunking is explicitly disabled
474+
// Should only be called once since chunking is disabled without timestampValueExpression
474475
expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledTimes(1);
475476
expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({
476477
config,
@@ -529,9 +530,12 @@ describe('useChartConfig', () => {
529530
.mockResolvedValueOnce(mockResponse2)
530531
.mockResolvedValueOnce(mockResponse3);
531532

532-
const { result } = renderHook(() => useQueriedChartConfig(config), {
533-
wrapper,
534-
});
533+
const { result } = renderHook(
534+
() => useChunkedQueriedChartConfig(config),
535+
{
536+
wrapper,
537+
},
538+
);
535539

536540
await waitFor(() => expect(result.current.isSuccess).toBe(true));
537541
await waitFor(() => expect(result.current.isFetching).toBe(false));
@@ -620,9 +624,12 @@ describe('useChartConfig', () => {
620624
.mockResolvedValueOnce(mockResponse2)
621625
.mockResolvedValueOnce(mockResponse3);
622626

623-
const { result } = renderHook(() => useQueriedChartConfig(config), {
624-
wrapper,
625-
});
627+
const { result } = renderHook(
628+
() => useChunkedQueriedChartConfig(config),
629+
{
630+
wrapper,
631+
},
632+
);
626633

627634
await waitFor(() => expect(result.current.isSuccess).toBe(true));
628635
await waitFor(() => expect(result.current.isPending).toBe(false));
@@ -678,9 +685,12 @@ describe('useChartConfig', () => {
678685
mockResponse1Promise,
679686
);
680687

681-
const { result } = renderHook(() => useQueriedChartConfig(config), {
682-
wrapper,
683-
});
688+
const { result } = renderHook(
689+
() => useChunkedQueriedChartConfig(config),
690+
{
691+
wrapper,
692+
},
693+
);
684694

685695
// Should be in loading state before first chunk
686696
expect(result.current.isLoading).toBe(true);
@@ -721,7 +731,11 @@ describe('useChartConfig', () => {
721731
});
722732

723733
const { result } = renderHook(
724-
() => useQueriedChartConfig(config, { onError, retry: false }),
734+
() =>
735+
useChunkedQueriedChartConfig(config, {
736+
onError,
737+
retry: false,
738+
}),
725739
{
726740
wrapper,
727741
},
@@ -752,7 +766,10 @@ describe('useChartConfig', () => {
752766
mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse);
753767

754768
const { result } = renderHook(
755-
() => useQueriedChartConfig(config, { enabled: false }),
769+
() =>
770+
useChunkedQueriedChartConfig(config, {
771+
enabled: false,
772+
}),
756773
{
757774
wrapper,
758775
},

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

Lines changed: 49 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,6 @@ import { generateTimeWindowsDescending } from '@/utils/searchWindows';
3030

3131
interface AdditionalUseQueriedChartConfigOptions {
3232
onError?: (error: Error | ClickHouseQueryError) => void;
33-
/**
34-
* By default, queries with large date ranges are split into multiple smaller queries to
35-
* avoid overloading the ClickHouse server and running into timeouts. In some cases, such
36-
* as when data is being sampled across the entire range, this chunking is not desirable
37-
* and can be disabled.
38-
*/
39-
disableQueryChunking?: boolean;
4033
}
4134

4235
type TimeWindow = {
@@ -64,7 +57,7 @@ export const getGranularityAlignedTimeWindows = (
6457
);
6558

6659
const granularity =
67-
config.granularity === 'auto' // TODO test this
60+
config.granularity === 'auto'
6861
? convertDateRangeToGranularityString(
6962
config.dateRange,
7063
DEFAULT_AUTO_GRANULARITY_MAX_BUCKETS,
@@ -102,24 +95,8 @@ async function* fetchDataInChunks(
10295
config: ChartConfigWithOptDateRange,
10396
clickhouseClient: ClickhouseClient,
10497
signal: AbortSignal,
105-
disableQueryChunking: boolean = false,
10698
) {
107-
const windows = disableQueryChunking
108-
? ([undefined] as const)
109-
: getGranularityAlignedTimeWindows(config);
110-
111-
let query = null;
112-
if (IS_MTVIEWS_ENABLED) {
113-
const { dataTableDDL, mtViewDDL, renderMTViewConfig } =
114-
await buildMTViewSelectQuery(config);
115-
// TODO: show the DDLs in the UI so users can run commands manually
116-
// eslint-disable-next-line no-console
117-
console.log('dataTableDDL:', dataTableDDL);
118-
// eslint-disable-next-line no-console
119-
console.log('mtViewDDL:', mtViewDDL);
120-
query = await renderMTViewConfig();
121-
}
122-
99+
const windows = getGranularityAlignedTimeWindows(config);
123100
for (const window of windows) {
124101
const windowedConfig = {
125102
...config,
@@ -141,18 +118,16 @@ async function* fetchDataInChunks(
141118
/**
142119
* A hook providing data queried based on the provided chart config.
143120
*
144-
* If all of the following are true, the query will be chunked into multiple smaller queries:
145-
* - The config includes a date range
146-
* - The config includes a granularity
147-
* - `options.disableQueryChunking` is falsy
121+
* The query will be chunked into multiple smaller queries if the config
122+
* includes a dateRange, granularity, timestampValueExpression.
148123
*
149124
* For chunked queries, note the following:
150-
* - The config's limit, if provided, is applied to each chunk, so the total number
125+
* - `config.limit`, if provided, is applied to each chunk, so the total number
151126
* of rows returned may be up to `limit * number_of_chunks`.
152127
* - The returned data will be ordered within each chunk, and chunks will
153-
* be ordered oldest-first, by the timestampValueExpression.
128+
* be ordered oldest-first, by the `timestampValueExpression`.
154129
*/
155-
export function useQueriedChartConfig(
130+
export function useChunkedQueriedChartConfig(
156131
config: ChartConfigWithOptDateRange,
157132
options?: Partial<UseQueryOptions<ResponseJSON<any>>> &
158133
AdditionalUseQueriedChartConfigOptions,
@@ -163,12 +138,7 @@ export function useQueriedChartConfig(
163138
queryKey: [config],
164139
queryFn: streamedQuery({
165140
streamFn: context =>
166-
fetchDataInChunks(
167-
config,
168-
clickhouseClient,
169-
context.signal,
170-
options?.disableQueryChunking,
171-
),
141+
fetchDataInChunks(config, clickhouseClient, context.signal),
172142
/**
173143
* This mode ensures that data remains in the cache until the next full streamed result is available.
174144
* By default, the cache would be cleared before new data starts arriving, which results in the query briefly
@@ -197,6 +167,47 @@ export function useQueriedChartConfig(
197167
return query;
198168
}
199169

170+
export function useQueriedChartConfig(
171+
config: ChartConfigWithOptDateRange,
172+
options?: Partial<UseQueryOptions<ResponseJSON<any>>> &
173+
AdditionalUseQueriedChartConfigOptions,
174+
) {
175+
const clickhouseClient = useClickhouseClient();
176+
const query = useQuery<ResponseJSON<any>, ClickHouseQueryError | Error>({
177+
queryKey: [config],
178+
queryFn: async ({ signal }) => {
179+
let query = null;
180+
if (IS_MTVIEWS_ENABLED) {
181+
const { dataTableDDL, mtViewDDL, renderMTViewConfig } =
182+
await buildMTViewSelectQuery(config);
183+
// TODO: show the DDLs in the UI so users can run commands manually
184+
// eslint-disable-next-line no-console
185+
console.log('dataTableDDL:', dataTableDDL);
186+
// eslint-disable-next-line no-console
187+
console.log('mtViewDDL:', mtViewDDL);
188+
query = await renderMTViewConfig();
189+
}
190+
191+
return clickhouseClient.queryChartConfig({
192+
config,
193+
metadata: getMetadata(),
194+
opts: {
195+
abort_signal: signal,
196+
},
197+
});
198+
},
199+
retry: 1,
200+
refetchOnWindowFocus: false,
201+
...options,
202+
});
203+
204+
if (query.isError && options?.onError) {
205+
options.onError(query.error);
206+
}
207+
208+
return query;
209+
}
210+
200211
export function useRenderedSqlChartConfig(
201212
config: ChartConfigWithOptDateRange,
202213
options?: UseQueryOptions<string>,

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,6 @@ function usePatterns({
145145
configWithPrimaryAndPartitionKey ?? config, // `config` satisfying type, never used due to `enabled` check
146146
{
147147
enabled: configWithPrimaryAndPartitionKey != null && enabled,
148-
// Disable chunking so that we get a uniform sample across the entire date range
149-
disableQueryChunking: true,
150148
},
151149
);
152150

0 commit comments

Comments
 (0)