Skip to content

Commit 383704b

Browse files
pulpdrewpulpdrew
committed
feat: Include displayed timestamp in default order by
1 parent 6587283 commit 383704b

File tree

4 files changed

+144
-29
lines changed

4 files changed

+144
-29
lines changed

‎.changeset/twelve-beers-buy.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+
feat: Include displayed timestamp in default order by

‎packages/app/src/DBSearchPage.tsx‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -532,14 +532,26 @@ function useSearchedConfigToChartConfig({
532532

533533
function optimizeDefaultOrderBy(
534534
timestampExpr: string,
535+
displayedTimestampExpr: string | undefined,
535536
sortingKey: string | undefined,
536537
) {
537538
const defaultModifier = 'DESC';
538-
const fallbackOrderByItems = [
539-
getFirstTimestampValueExpression(timestampExpr ?? ''),
540-
defaultModifier,
541-
];
542-
const fallbackOrderBy = fallbackOrderByItems.join(' ');
539+
const firstTimestampValueExpression =
540+
getFirstTimestampValueExpression(timestampExpr ?? '') ?? '';
541+
const defaultOrderByItems = [firstTimestampValueExpression];
542+
const trimmedDisplayedTimestampExpr = displayedTimestampExpr?.trim();
543+
544+
if (
545+
trimmedDisplayedTimestampExpr &&
546+
trimmedDisplayedTimestampExpr !== firstTimestampValueExpression
547+
) {
548+
defaultOrderByItems.push(trimmedDisplayedTimestampExpr);
549+
}
550+
551+
const fallbackOrderBy =
552+
defaultOrderByItems.length > 1
553+
? `(${defaultOrderByItems.join(', ')}) ${defaultModifier}`
554+
: `${defaultOrderByItems[0]} ${defaultModifier}`;
543555

544556
if (!sortingKey) return fallbackOrderBy;
545557

@@ -550,10 +562,11 @@ function optimizeDefaultOrderBy(
550562
if (sortKey.includes('toStartOf') && sortKey.includes(timestampExpr)) {
551563
orderByArr.push(sortKey);
552564
} else if (
553-
sortKey === timestampExpr ||
565+
sortKey === firstTimestampValueExpression ||
554566
(sortKey.startsWith('toUnixTimestamp') &&
555-
sortKey.includes(timestampExpr)) ||
556-
(sortKey.startsWith('toDateTime') && sortKey.includes(timestampExpr))
567+
sortKey.includes(firstTimestampValueExpression)) ||
568+
(sortKey.startsWith('toDateTime') &&
569+
sortKey.includes(firstTimestampValueExpression))
557570
) {
558571
if (orderByArr.length === 0) {
559572
// fallback if the first sort key is the timestamp sort key
@@ -562,6 +575,8 @@ function optimizeDefaultOrderBy(
562575
orderByArr.push(sortKey);
563576
break;
564577
}
578+
} else if (sortKey === trimmedDisplayedTimestampExpr) {
579+
orderByArr.push(sortKey);
565580
}
566581
}
567582

@@ -570,7 +585,16 @@ function optimizeDefaultOrderBy(
570585
return fallbackOrderBy;
571586
}
572587

573-
return `(${orderByArr.join(', ')}) ${defaultModifier}`;
588+
if (
589+
trimmedDisplayedTimestampExpr &&
590+
!orderByArr.includes(trimmedDisplayedTimestampExpr)
591+
) {
592+
orderByArr.push(trimmedDisplayedTimestampExpr);
593+
}
594+
595+
return orderByArr.length > 1
596+
? `(${orderByArr.join(', ')}) ${defaultModifier}`
597+
: `${orderByArr[0]} ${defaultModifier}`;
574598
}
575599

576600
export function useDefaultOrderBy(sourceID: string | undefined | null) {
@@ -582,6 +606,7 @@ export function useDefaultOrderBy(sourceID: string | undefined | null) {
582606
() =>
583607
optimizeDefaultOrderBy(
584608
source?.timestampValueExpression ?? '',
609+
source?.displayedTimestampValueExpression,
585610
tableMetadata?.sorting_key,
586611
),
587612
[source, tableMetadata],

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

Lines changed: 89 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,82 +17,152 @@ describe('useDefaultOrderBy', () => {
1717

1818
describe('optimizeOrderBy function', () => {
1919
describe('should handle', () => {
20-
const mockSource = {
21-
timestampValueExpression: 'Timestamp',
22-
};
23-
2420
const testCases = [
2521
{
26-
input: undefined,
22+
sortingKey: undefined,
2723
expected: 'Timestamp DESC',
2824
},
2925
{
30-
input: '',
26+
sortingKey: '',
3127
expected: 'Timestamp DESC',
3228
},
3329
{
3430
// Traces Table
35-
input: 'ServiceName, SpanName, toDateTime(Timestamp)',
31+
sortingKey: 'ServiceName, SpanName, toDateTime(Timestamp)',
3632
expected: 'Timestamp DESC',
3733
},
3834
{
3935
// Optimized Traces Table
40-
input:
36+
sortingKey:
4137
'toStartOfHour(Timestamp), ServiceName, SpanName, toDateTime(Timestamp)',
4238
expected: '(toStartOfHour(Timestamp), toDateTime(Timestamp)) DESC',
4339
},
4440
{
4541
// Unsupported for now as it's not a great sort key, want to just
4642
// use default behavior for this
47-
input: 'toDateTime(Timestamp), ServiceName, SpanName, Timestamp',
43+
sortingKey: 'toDateTime(Timestamp), ServiceName, SpanName, Timestamp',
4844
expected: 'Timestamp DESC',
4945
},
5046
{
5147
// Unsupported prefix sort key
52-
input: 'toDateTime(Timestamp), ServiceName, SpanName',
48+
sortingKey: 'toDateTime(Timestamp), ServiceName, SpanName',
5349
expected: 'Timestamp DESC',
5450
},
5551
{
5652
// Inverted sort key order, we should not try to optimize this
57-
input:
53+
sortingKey:
5854
'ServiceName, toDateTime(Timestamp), SeverityText, toStartOfHour(Timestamp)',
5955
expected: 'Timestamp DESC',
6056
},
6157
{
62-
input: 'toStartOfHour(Timestamp), other_column, Timestamp',
58+
sortingKey: 'toStartOfHour(Timestamp), other_column, Timestamp',
6359
expected: '(toStartOfHour(Timestamp), Timestamp) DESC',
6460
},
6561
{
66-
input: 'Timestamp, other_column',
62+
sortingKey: 'Timestamp, other_column',
6763
expected: 'Timestamp DESC',
6864
},
6965
{
70-
input: 'user_id, toStartOfHour(Timestamp), status, Timestamp',
66+
sortingKey: 'user_id, toStartOfHour(Timestamp), status, Timestamp',
7167
expected: '(toStartOfHour(Timestamp), Timestamp) DESC',
7268
},
7369
{
74-
input:
70+
sortingKey:
7571
'toStartOfMinute(Timestamp), user_id, status, toUnixTimestamp(Timestamp)',
7672
expected:
7773
'(toStartOfMinute(Timestamp), toUnixTimestamp(Timestamp)) DESC',
7874
},
7975
{
8076
// test variation of toUnixTimestamp
81-
input:
77+
sortingKey:
8278
'toStartOfMinute(Timestamp), user_id, status, toUnixTimestamp64Nano(Timestamp)',
8379
expected:
8480
'(toStartOfMinute(Timestamp), toUnixTimestamp64Nano(Timestamp)) DESC',
8581
},
8682
{
87-
input:
83+
sortingKey:
8884
'toUnixTimestamp(toStartOfMinute(Timestamp)), user_id, status, Timestamp',
8985
expected:
9086
'(toUnixTimestamp(toStartOfMinute(Timestamp)), Timestamp) DESC',
9187
},
88+
{
89+
sortingKey: 'Timestamp',
90+
displayedTimestampValueExpression: 'Timestamp64',
91+
expected: '(Timestamp, Timestamp64) DESC',
92+
},
93+
{
94+
sortingKey: 'Timestamp',
95+
displayedTimestampValueExpression: 'Timestamp64 ',
96+
expected: '(Timestamp, Timestamp64) DESC',
97+
},
98+
{
99+
sortingKey: 'Timestamp',
100+
displayedTimestampValueExpression: 'Timestamp',
101+
expected: 'Timestamp DESC',
102+
},
103+
{
104+
sortingKey: 'Timestamp',
105+
displayedTimestampValueExpression: '',
106+
expected: 'Timestamp DESC',
107+
},
108+
{
109+
sortingKey: 'Timestamp, ServiceName, Timestamp64',
110+
displayedTimestampValueExpression: 'Timestamp64',
111+
expected: '(Timestamp, Timestamp64) DESC',
112+
},
113+
{
114+
sortingKey:
115+
'toStartOfMinute(Timestamp), Timestamp, ServiceName, Timestamp64',
116+
displayedTimestampValueExpression: 'Timestamp64',
117+
expected: '(toStartOfMinute(Timestamp), Timestamp, Timestamp64) DESC',
118+
},
119+
{
120+
sortingKey:
121+
'toStartOfMinute(Timestamp), Timestamp64, ServiceName, Timestamp',
122+
displayedTimestampValueExpression: 'Timestamp64',
123+
expected: '(toStartOfMinute(Timestamp), Timestamp64, Timestamp) DESC',
124+
},
125+
{
126+
sortingKey: 'SomeOtherTimeColumn',
127+
displayedTimestampValueExpression: 'Timestamp64',
128+
expected: '(Timestamp, Timestamp64) DESC',
129+
},
130+
{
131+
sortingKey: '',
132+
displayedTimestampValueExpression: 'Timestamp64',
133+
expected: '(Timestamp, Timestamp64) DESC',
134+
},
135+
{
136+
sortingKey: 'ServiceName, TimestampTime, Timestamp',
137+
timestampValueExpression: 'TimestampTime, Timestamp',
138+
displayedTimestampValueExpression: 'Timestamp',
139+
expected: '(TimestampTime, Timestamp) DESC',
140+
},
141+
{
142+
sortingKey: 'ServiceName, TimestampTime, Timestamp',
143+
timestampValueExpression: 'Timestamp, TimestampTime',
144+
displayedTimestampValueExpression: 'Timestamp',
145+
expected: 'Timestamp DESC',
146+
},
147+
{
148+
sortingKey: '',
149+
timestampValueExpression: 'Timestamp, TimestampTime',
150+
displayedTimestampValueExpression: '',
151+
expected: 'Timestamp DESC',
152+
},
92153
];
93154
for (const testCase of testCases) {
94-
it(`${testCase.input}`, () => {
95-
const mockTableMetadata = { sorting_key: testCase.input };
155+
it(`${testCase.sortingKey}`, () => {
156+
const mockSource = {
157+
timestampValueExpression:
158+
testCase.timestampValueExpression || 'Timestamp',
159+
displayedTimestampValueExpression:
160+
testCase.displayedTimestampValueExpression,
161+
};
162+
163+
const mockTableMetadata = {
164+
sorting_key: testCase.sortingKey,
165+
};
96166

97167
jest.spyOn(sourceModule, 'useSource').mockReturnValue({
98168
data: mockSource,

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ export function LogTableModelForm({ control, watch }: TableModelProps) {
285285
</FormRow>
286286
<FormRow
287287
label={'Displayed Timestamp Column'}
288-
helpText="This DateTime column is used to display search results."
288+
helpText="This DateTime column is used to display and order search results."
289289
>
290290
<SQLInlineEditorControlled
291291
tableConnection={{
@@ -631,6 +631,21 @@ export function TraceTableModelForm({ control, watch }: TableModelProps) {
631631
placeholder="SpanName"
632632
/>
633633
</FormRow>
634+
<FormRow
635+
label={'Displayed Timestamp Column'}
636+
helpText="This DateTime column is used to display and order search results."
637+
>
638+
<SQLInlineEditorControlled
639+
tableConnection={{
640+
databaseName,
641+
tableName,
642+
connectionId,
643+
}}
644+
control={control}
645+
name="displayedTimestampValueExpression"
646+
disableKeywordAutocomplete
647+
/>
648+
</FormRow>
634649
</Stack>
635650
);
636651
}

0 commit comments

Comments
 (0)