Skip to content

Commit 7445bbf

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

File tree

4 files changed

+153
-30
lines changed

4 files changed

+153
-30
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: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -532,28 +532,44 @@ 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

546558
const orderByArr = [];
547559
const sortKeys = splitAndTrimWithBracket(sortingKey);
548560
for (let i = 0; i < sortKeys.length; i++) {
549561
const sortKey = sortKeys[i];
550-
if (sortKey.includes('toStartOf') && sortKey.includes(timestampExpr)) {
562+
if (
563+
sortKey.includes('toStartOf') &&
564+
sortKey.includes(firstTimestampValueExpression)
565+
) {
551566
orderByArr.push(sortKey);
552567
} else if (
553-
sortKey === timestampExpr ||
568+
sortKey === firstTimestampValueExpression ||
554569
(sortKey.startsWith('toUnixTimestamp') &&
555-
sortKey.includes(timestampExpr)) ||
556-
(sortKey.startsWith('toDateTime') && sortKey.includes(timestampExpr))
570+
sortKey.includes(firstTimestampValueExpression)) ||
571+
(sortKey.startsWith('toDateTime') &&
572+
sortKey.includes(firstTimestampValueExpression))
557573
) {
558574
if (orderByArr.length === 0) {
559575
// fallback if the first sort key is the timestamp sort key
@@ -562,6 +578,8 @@ function optimizeDefaultOrderBy(
562578
orderByArr.push(sortKey);
563579
break;
564580
}
581+
} else if (sortKey === trimmedDisplayedTimestampExpr) {
582+
orderByArr.push(sortKey);
565583
}
566584
}
567585

@@ -570,7 +588,16 @@ function optimizeDefaultOrderBy(
570588
return fallbackOrderBy;
571589
}
572590

573-
return `(${orderByArr.join(', ')}) ${defaultModifier}`;
591+
if (
592+
trimmedDisplayedTimestampExpr &&
593+
!orderByArr.includes(trimmedDisplayedTimestampExpr)
594+
) {
595+
orderByArr.push(trimmedDisplayedTimestampExpr);
596+
}
597+
598+
return orderByArr.length > 1
599+
? `(${orderByArr.join(', ')}) ${defaultModifier}`
600+
: `${orderByArr[0]} ${defaultModifier}`;
574601
}
575602

576603
export function useDefaultOrderBy(sourceID: string | undefined | null) {
@@ -582,6 +609,7 @@ export function useDefaultOrderBy(sourceID: string | undefined | null) {
582609
() =>
583610
optimizeDefaultOrderBy(
584611
source?.timestampValueExpression ?? '',
612+
source?.displayedTimestampValueExpression,
585613
tableMetadata?.sorting_key,
586614
),
587615
[source, tableMetadata],

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

Lines changed: 94 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,82 +17,157 @@ 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: 'toStartOfMinute(Timestamp), user_id, status, Timestamp',
90+
timestampValueExpression: 'Timestamp, toStartOfMinute(Timestamp)',
91+
expected: '(toStartOfMinute(Timestamp), Timestamp) DESC',
92+
},
93+
{
94+
sortingKey: 'Timestamp',
95+
displayedTimestampValueExpression: 'Timestamp64',
96+
expected: '(Timestamp, Timestamp64) DESC',
97+
},
98+
{
99+
sortingKey: 'Timestamp',
100+
displayedTimestampValueExpression: 'Timestamp64 ',
101+
expected: '(Timestamp, Timestamp64) DESC',
102+
},
103+
{
104+
sortingKey: 'Timestamp',
105+
displayedTimestampValueExpression: 'Timestamp',
106+
expected: 'Timestamp DESC',
107+
},
108+
{
109+
sortingKey: 'Timestamp',
110+
displayedTimestampValueExpression: '',
111+
expected: 'Timestamp DESC',
112+
},
113+
{
114+
sortingKey: 'Timestamp, ServiceName, Timestamp64',
115+
displayedTimestampValueExpression: 'Timestamp64',
116+
expected: '(Timestamp, Timestamp64) DESC',
117+
},
118+
{
119+
sortingKey:
120+
'toStartOfMinute(Timestamp), Timestamp, ServiceName, Timestamp64',
121+
displayedTimestampValueExpression: 'Timestamp64',
122+
expected: '(toStartOfMinute(Timestamp), Timestamp, Timestamp64) DESC',
123+
},
124+
{
125+
sortingKey:
126+
'toStartOfMinute(Timestamp), Timestamp64, ServiceName, Timestamp',
127+
displayedTimestampValueExpression: 'Timestamp64',
128+
expected: '(toStartOfMinute(Timestamp), Timestamp64, Timestamp) DESC',
129+
},
130+
{
131+
sortingKey: 'SomeOtherTimeColumn',
132+
displayedTimestampValueExpression: 'Timestamp64',
133+
expected: '(Timestamp, Timestamp64) DESC',
134+
},
135+
{
136+
sortingKey: '',
137+
displayedTimestampValueExpression: 'Timestamp64',
138+
expected: '(Timestamp, Timestamp64) DESC',
139+
},
140+
{
141+
sortingKey: 'ServiceName, TimestampTime, Timestamp',
142+
timestampValueExpression: 'TimestampTime, Timestamp',
143+
displayedTimestampValueExpression: 'Timestamp',
144+
expected: '(TimestampTime, Timestamp) DESC',
145+
},
146+
{
147+
sortingKey: 'ServiceName, TimestampTime, Timestamp',
148+
timestampValueExpression: 'Timestamp, TimestampTime',
149+
displayedTimestampValueExpression: 'Timestamp',
150+
expected: 'Timestamp DESC',
151+
},
152+
{
153+
sortingKey: '',
154+
timestampValueExpression: 'Timestamp, TimestampTime',
155+
displayedTimestampValueExpression: '',
156+
expected: 'Timestamp DESC',
157+
},
92158
];
93159
for (const testCase of testCases) {
94-
it(`${testCase.input}`, () => {
95-
const mockTableMetadata = { sorting_key: testCase.input };
160+
it(`${testCase.sortingKey}`, () => {
161+
const mockSource = {
162+
timestampValueExpression:
163+
testCase.timestampValueExpression || 'Timestamp',
164+
displayedTimestampValueExpression:
165+
testCase.displayedTimestampValueExpression,
166+
};
167+
168+
const mockTableMetadata = {
169+
sorting_key: testCase.sortingKey,
170+
};
96171

97172
jest.spyOn(sourceModule, 'useSource').mockReturnValue({
98173
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)