Skip to content

Commit 35fe9cf

Browse files
authored
fix: for order by optimization (#1075)
Ref HDX-1955
1 parent 5a59d32 commit 35fe9cf

File tree

3 files changed

+32
-23
lines changed

3 files changed

+32
-23
lines changed

.changeset/metal-trainers-sniff.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+
fix default order by generated for advanced table sorting keys

packages/app/src/DBSearchPage.tsx

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -498,24 +498,31 @@ function useSearchedConfigToChartConfig({
498498
]);
499499
}
500500

501-
function optimizeOrderBy(
501+
function optimizeDefaultOrderBy(
502502
timestampExpr: string,
503-
orderBy: string,
504-
sortingKey: string,
503+
sortingKey: string | undefined,
505504
) {
505+
const defaultModifier = 'DESC';
506+
const fallbackOrderByItems = [
507+
getFirstTimestampValueExpression(timestampExpr ?? ''),
508+
defaultModifier,
509+
];
510+
const fallbackOrderBy = fallbackOrderByItems.join(' ');
511+
if (!sortingKey) return fallbackOrderBy;
512+
506513
const sortKeys = sortingKey.split(',').map(key => key.trim());
507514
const timestampExprIdx = sortKeys.findIndex(v => v === timestampExpr);
508-
if (timestampExprIdx <= 0) return orderBy;
509-
const orderByArr = [orderBy];
515+
if (timestampExprIdx <= 0) return fallbackOrderBy;
510516

517+
const orderByArr = [fallbackOrderByItems[0]];
511518
for (let i = 0; i < timestampExprIdx; i++) {
512519
const sortKey = sortKeys[i];
513520
if (sortKey.includes('toStartOf') && sortKey.includes(timestampExpr)) {
514521
orderByArr.push(sortKey);
515522
}
516523
}
517524

518-
const newOrderBy = orderByArr.reverse().join(', ');
525+
const newOrderBy = `(${orderByArr.reverse().join(', ')}) ${defaultModifier}`;
519526
return newOrderBy;
520527
}
521528

@@ -524,17 +531,14 @@ export function useDefaultOrderBy(sourceID: string | undefined | null) {
524531
const { data: tableMetadata } = useTableMetadata(tcFromSource(source));
525532

526533
// When source changes, make sure select and orderby fields are set to default
527-
return useMemo(() => {
528-
const fallbackOrderBy = `${getFirstTimestampValueExpression(
529-
source?.timestampValueExpression ?? '',
530-
)} DESC`;
531-
if (!tableMetadata) return fallbackOrderBy;
532-
return optimizeOrderBy(
533-
source?.timestampValueExpression ?? '',
534-
fallbackOrderBy,
535-
tableMetadata.sorting_key,
536-
);
537-
}, [source, tableMetadata]);
534+
return useMemo(
535+
() =>
536+
optimizeDefaultOrderBy(
537+
source?.timestampValueExpression ?? '',
538+
tableMetadata?.sorting_key,
539+
),
540+
[source, tableMetadata],
541+
);
538542
}
539543

540544
// This is outside as it needs to be a stable reference

packages/app/src/__tests__/DBSearchPage.test.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describe('useDefaultOrderBy', () => {
5757

5858
const { result } = renderHook(() => useDefaultOrderBy('source-id'));
5959

60-
expect(result.current).toBe('undefined DESC');
60+
expect(result.current).toBe(' DESC');
6161
});
6262

6363
it('should return optimized order by when Timestamp is not first in sorting key', () => {
@@ -83,7 +83,7 @@ describe('useDefaultOrderBy', () => {
8383

8484
const { result } = renderHook(() => useDefaultOrderBy('source-id'));
8585

86-
expect(result.current).toBe('toStartOfHour(Timestamp), Timestamp DESC');
86+
expect(result.current).toBe('(toStartOfHour(Timestamp), Timestamp) DESC');
8787
});
8888

8989
it('should return fallback when Timestamp is first in sorting key', () => {
@@ -135,7 +135,7 @@ describe('useDefaultOrderBy', () => {
135135

136136
const { result } = renderHook(() => useDefaultOrderBy('source-id'));
137137

138-
expect(result.current).toBe('toStartOfHour(Timestamp), Timestamp DESC');
138+
expect(result.current).toBe('(toStartOfHour(Timestamp), Timestamp) DESC');
139139
});
140140

141141
it('should handle null source ungracefully', () => {
@@ -153,7 +153,7 @@ describe('useDefaultOrderBy', () => {
153153

154154
const { result } = renderHook(() => useDefaultOrderBy(null));
155155

156-
expect(result.current).toBe('undefined DESC');
156+
expect(result.current).toBe(' DESC');
157157
});
158158

159159
it('should handle undefined sourceID ungracefully', () => {
@@ -171,7 +171,7 @@ describe('useDefaultOrderBy', () => {
171171

172172
const { result } = renderHook(() => useDefaultOrderBy(undefined));
173173

174-
expect(result.current).toBe('undefined DESC');
174+
expect(result.current).toBe(' DESC');
175175
});
176176

177177
it('should handle complex Timestamp expressions', () => {
@@ -199,7 +199,7 @@ describe('useDefaultOrderBy', () => {
199199
const { result } = renderHook(() => useDefaultOrderBy('source-id'));
200200

201201
expect(result.current).toBe(
202-
'toStartOfHour(toDateTime(timestamp_ms / 1000)), toDateTime(timestamp_ms / 1000) DESC',
202+
'(toStartOfHour(toDateTime(timestamp_ms / 1000)), toDateTime(timestamp_ms / 1000)) DESC',
203203
);
204204
});
205205

0 commit comments

Comments
 (0)