Skip to content

Commit 609ab14

Browse files
authored
chore: Update queryTableMetadata type to include undefined (#1939)
## Summary This change updates the return type of queryTableMetadata (and therefore `getTableMetadata()` and `useTableMetadata()`) to include `undefined`, since the lookup can return undefined when the provided table does not exist. This required adding undefined checks at a few call sites. ### Screenshots or video There are no behavior changes expected, other than fewer error logs on a few pages where CTEs are used (eg. K8s dashboard). ### How to test locally or on Vercel This can be tested in the preview environment. ### References - Linear Issue: n/a - Related PRs:
1 parent 72d4642 commit 609ab14

7 files changed

Lines changed: 39 additions & 36 deletions

File tree

packages/app/src/hooks/useMetadata.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ export function useTableMetadata(
190190
options?: Omit<UseQueryOptions<any, Error>, 'queryKey'>,
191191
) {
192192
const metadata = useMetadataWithSettings();
193-
return useQuery<TableMetadata>({
193+
return useQuery<TableMetadata | undefined>({
194194
queryKey: ['useMetadata.useTableMetadata', { databaseName, tableName }],
195195
queryFn: async () => {
196196
return await metadata.getTableMetadata({

packages/app/src/source.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,10 @@ export async function inferTableSourceConfig({
208208
tableName,
209209
connectionId,
210210
})
211-
).primary_key;
212-
const primaryKeyColumns = new Set(
213-
extractColumnReferencesFromKey(primaryKeys),
214-
);
211+
)?.primary_key;
212+
const primaryKeyColumns = primaryKeys
213+
? new Set(extractColumnReferencesFromKey(primaryKeys))
214+
: new Set();
215215

216216
const isOtelLogSchema = hasAllColumns(columns, [
217217
'Timestamp',

packages/app/src/utils/materializedViews.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import {
1818
MaterializedViewConfiguration,
1919
} from '@hyperdx/common-utils/dist/types';
2020

21-
import { getMetadata } from '@/metadata';
22-
2321
export const MV_AGGREGATE_FUNCTIONS = [
2422
'avg',
2523
'count',
@@ -78,16 +76,22 @@ const isAggregateFn = (
7876
return MV_AGGREGATE_FUNCTIONS.includes(aggFn ?? '');
7977
};
8078

81-
function isMaterializedView(meta: TableMetadata) {
82-
return meta.engine?.startsWith('MaterializedView') ?? false;
79+
function isMaterializedView(
80+
meta: TableMetadata | undefined,
81+
): meta is TableMetadata {
82+
return meta?.engine?.startsWith('MaterializedView') ?? false;
8383
}
8484

85-
function isAggregatingMergeTree(meta: TableMetadata) {
86-
return meta.engine?.includes('AggregatingMergeTree') ?? false;
85+
function isAggregatingMergeTree(
86+
meta: TableMetadata | undefined,
87+
): meta is TableMetadata {
88+
return meta?.engine?.includes('AggregatingMergeTree') ?? false;
8789
}
8890

89-
function isSummingMergeTree(meta: TableMetadata) {
90-
return meta.engine?.includes('SummingMergeTree') ?? false;
91+
function isSummingMergeTree(
92+
meta: TableMetadata | undefined,
93+
): meta is TableMetadata {
94+
return meta?.engine?.includes('SummingMergeTree') ?? false;
9195
}
9296

9397
/**

packages/common-utils/src/__tests__/metadata.int.test.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -574,20 +574,20 @@ describe('Metadata Integration Tests', () => {
574574
});
575575

576576
// Should have the distributed table's create_table_query
577-
expect(result.create_table_query).toContain(distributedTableName);
578-
expect(result.create_table_query).toContain('Distributed');
577+
expect(result!.create_table_query).toContain(distributedTableName);
578+
expect(result!.create_table_query).toContain('Distributed');
579579

580580
// Should have the local table's create_table_query
581-
expect(result.create_local_table_query).toContain(localTableName);
582-
expect(result.create_local_table_query).toContain('MergeTree');
581+
expect(result!.create_local_table_query).toContain(localTableName);
582+
expect(result!.create_local_table_query).toContain('MergeTree');
583583

584584
// Keys should come from the local table
585-
expect(result.primary_key).toBe('ServiceName, Timestamp');
586-
expect(result.sorting_key).toBe('ServiceName, Timestamp');
587-
expect(result.partition_key).toBe('toDate(Timestamp)');
585+
expect(result!.primary_key).toBe('ServiceName, Timestamp');
586+
expect(result!.sorting_key).toBe('ServiceName, Timestamp');
587+
expect(result!.partition_key).toBe('toDate(Timestamp)');
588588

589589
// Engine should be overridden with local table's engine
590-
expect(result.engine).toBe('MergeTree');
590+
expect(result!.engine).toBe('MergeTree');
591591
});
592592

593593
it('should not set create_local_table_query for non-distributed tables', async () => {
@@ -597,9 +597,9 @@ describe('Metadata Integration Tests', () => {
597597
connectionId: 'test_connection',
598598
});
599599

600-
expect(result.create_local_table_query).toBeUndefined();
601-
expect(result.engine).toBe('MergeTree');
602-
expect(result.primary_key).toBe('ServiceName, Timestamp');
600+
expect(result!.create_local_table_query).toBeUndefined();
601+
expect(result!.engine).toBe('MergeTree');
602+
expect(result!.primary_key).toBe('ServiceName, Timestamp');
603603
});
604604

605605
it('should return skip indices from the local table when querying a distributed table', async () => {

packages/common-utils/src/__tests__/metadata.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ describe('MetadataCache', () => {
119119

120120
try {
121121
await metadataCache.getOrFetch(key, queryFn);
122-
} catch (e) {
122+
} catch {
123123
// Expected to throw
124124
}
125125

@@ -165,7 +165,7 @@ describe('Metadata', () => {
165165
connectionId: 'test_connection',
166166
});
167167

168-
expect(result.partition_key).toEqual('toYYYYMM(timestamp), user_id');
168+
expect(result!.partition_key).toEqual('toYYYYMM(timestamp), user_id');
169169
});
170170

171171
it('should not modify partition_key if it does not have parentheses', async () => {
@@ -189,7 +189,7 @@ describe('Metadata', () => {
189189
connectionId: 'test_connection',
190190
});
191191

192-
expect(result.partition_key).toEqual('column1');
192+
expect(result!.partition_key).toEqual('column1');
193193
});
194194

195195
it('should use the cache when retrieving table metadata', async () => {

packages/common-utils/src/core/metadata.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export class Metadata {
153153
table: string;
154154
cache: MetadataCache;
155155
connectionId: string;
156-
}) {
156+
}): Promise<TableMetadata | undefined> {
157157
return cache.getOrFetch(
158158
`${connectionId}.${database}.${table}.metadata`,
159159
async () => {
@@ -241,7 +241,7 @@ export class Metadata {
241241
connectionId,
242242
});
243243

244-
// Build up materalized fields lookup table
244+
// Build up materialized fields lookup table
245245
return new Map(
246246
columns
247247
.filter(
@@ -617,7 +617,7 @@ export class Metadata {
617617
});
618618

619619
// For Distributed tables, fetch metadata of the underlying local table to get correct partition key, sorting key, etc.
620-
if (tableMetadata.engine === 'Distributed') {
620+
if (tableMetadata?.engine === 'Distributed') {
621621
try {
622622
const { database, table } =
623623
getLocalTableFromDistributedTable(tableMetadata) ?? {};
@@ -649,7 +649,7 @@ export class Metadata {
649649
'primary_key',
650650
'sampling_key',
651651
]),
652-
create_local_table_query: localTableMetadata.create_table_query,
652+
create_local_table_query: localTableMetadata?.create_table_query,
653653
};
654654
} catch (e) {
655655
console.error(
@@ -661,7 +661,7 @@ export class Metadata {
661661

662662
// partition_key which includes parenthesis, unlike other keys such as 'primary_key' or 'sorting_key'
663663
if (
664-
tableMetadata.partition_key.startsWith('(') &&
664+
tableMetadata?.partition_key.startsWith('(') &&
665665
tableMetadata.partition_key.endsWith(')')
666666
) {
667667
tableMetadata.partition_key = tableMetadata.partition_key.slice(1, -1);
@@ -777,7 +777,7 @@ export class Metadata {
777777
let localTable = tableName;
778778

779779
// For Distributed tables, fetch skip indices on the underlying local table.
780-
if (tableMetadata.engine === 'Distributed') {
780+
if (tableMetadata?.engine === 'Distributed') {
781781
try {
782782
const { database, table } =
783783
getLocalTableFromDistributedTable(tableMetadata) ?? {};

packages/common-utils/src/core/renderChartConfig.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import {
3131
CteChartConfig,
3232
DateRange,
3333
DisplayType,
34-
Filter,
3534
MetricsDataType,
3635
QuerySettings,
3736
RawSqlChartConfig,
@@ -577,14 +576,14 @@ export async function timeFilterExpr({
577576
try {
578577
// Not all of these will be available when selecting from a CTE
579578
if (databaseName && tableName && connectionId) {
580-
const { primary_key } = await metadata.getTableMetadata({
579+
const tableMetadata = await metadata.getTableMetadata({
581580
databaseName,
582581
tableName,
583582
connectionId,
584583
});
585584
optimizedTimestampValueExpression = optimizeTimestampValueExpression(
586585
timestampValueExpression,
587-
primary_key,
586+
tableMetadata?.primary_key,
588587
);
589588
}
590589
} catch (e) {

0 commit comments

Comments
 (0)