Elgato_dark/hyperdx
1
0
Fork 0
mirror of https://github.com/hyperdxio/hyperdx synced 2026-04-21 13:37:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

refactor: clean up the chart config CTE render logic (#686)

Browse source 
Some additional refactoring and testing around the more complex CTE rendering.

Ref: HDX-1511
This commit is contained in:
Dan Hable 2025-03-17 09:45:26 -05:00 committed by GitHub
parent bbdc53457a
commit b9f7d32efa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 190 additions and 88 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
.changeset/moody-chefs-juggle.md Normal file
View file

@ -0,0 +1,6 @@
---
"@hyperdx/common-utils": patch
"@hyperdx/api": patch
---
Refactored renderWith to simplify logic and ship more tests with the changes.

2
packages/api/src/clickhouse/__tests__/renderChartConfig.test.ts
View file

@ -133,7 +133,7 @@ describe('renderChartConfig', () => {
query_params: query.params,
format: 'JSON',
})
.then(res => res.json<any>());
.then(res => res.json() as any);
expect(resp.data).toMatchSnapshot();
});

8
packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap
View file

@ -1,5 +1,9 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`renderChartConfig containing CTE clauses should render a ChSql CTE configuration correctly 1`] = `"WITH TestCte AS (SELECT TimeUnix, Line FROM otel_logs) SELECT Line FROM TestCte"`;
exports[`renderChartConfig containing CTE clauses should render a chart config CTE configuration correctly 1`] = `"WITH Parts AS (SELECT _part, _part_offset FROM default.some_table WHERE ((FieldA = 'test')) ORDER BY rand() DESC LIMIT 1000) SELECT * FROM Parts WHERE ((FieldA = 'test') AND (indexHint((_part, _part_offset) IN (SELECT tuple(_part, _part_offset) FROM Parts)))) ORDER BY rand() DESC LIMIT 1000"`;
exports[`renderChartConfig should generate sql for a single gauge metric 1`] = `
"WITH Source AS (
SELECT
@ -107,7 +111,3 @@ exports[`renderChartConfig should generate sql for a single sum metric 1`] = `
toFloat64OrNull(toString(Value))
),toStartOfInterval(toDateTime(\`__hdx_time_bucket2\`), INTERVAL 5 minute) AS \`__hdx_time_bucket\` FROM Bucketed WHERE (\`__hdx_time_bucket2\` >= fromUnixTimestamp64Milli(1739318400000) AND \`__hdx_time_bucket2\` <= fromUnixTimestamp64Milli(1765670400000)) GROUP BY toStartOfInterval(toDateTime(\`__hdx_time_bucket2\`), INTERVAL 5 minute) AS \`__hdx_time_bucket\` ORDER BY toStartOfInterval(toDateTime(\`__hdx_time_bucket2\`), INTERVAL 5 minute) AS \`__hdx_time_bucket\` LIMIT 10"
`;
exports[`renderChartConfig should render a chart config CTE configuration correctly 1`] = `"WITH Parts AS (SELECT _part, _part_offset FROM default.some_table WHERE ((FieldA = 'test')) ORDER BY rand() DESC LIMIT 1000) SELECT * FROM Parts WHERE ((FieldA = 'test') AND (indexHint((_part, _part_offset) IN (SELECT tuple(_part, _part_offset) FROM Parts)))) ORDER BY rand() DESC LIMIT 1000"`;
exports[`renderChartConfig should render a string CTE configuration correctly 1`] = `"WITH TestCte AS (SELECT TimeUnix, Line FROM otel_logs) SELECT Line FROM TestCte"`;

198
packages/common-utils/src/__tests__/renderChartConfig.test.ts
View file

@ -1,4 +1,4 @@
import { parameterizedQueryToSql } from '@/clickhouse';
import { chSql, parameterizedQueryToSql } from '@/clickhouse';
import { Metadata } from '@/metadata';
import {
ChartConfigWithOptDateRange,
@ -159,69 +159,147 @@ describe('renderChartConfig', () => {
expect(actual).toMatchSnapshot();
});
it('should render a string CTE configuration correctly', async () => {
const config: ChartConfigWithOptDateRange = {
connection: 'test-connection',
from: {
databaseName: '',
tableName: 'TestCte',
},
with: [{ name: 'TestCte', sql: 'SELECT TimeUnix, Line FROM otel_logs' }],
select: [{ valueExpression: 'Line' }],
where: '',
whereLanguage: 'sql',
};
describe('containing CTE clauses', () => {
it('should render a ChSql CTE configuration correctly', async () => {
const config: ChartConfigWithOptDateRange = {
connection: 'test-connection',
from: {
databaseName: '',
tableName: 'TestCte',
},
with: [
{ name: 'TestCte', sql: chSql`SELECT TimeUnix, Line FROM otel_logs` },
],
select: [{ valueExpression: 'Line' }],
where: '',
whereLanguage: 'sql',
};
const generatedSql = await renderChartConfig(config, mockMetadata);
const actual = parameterizedQueryToSql(generatedSql);
expect(actual).toMatchSnapshot();
});
const generatedSql = await renderChartConfig(config, mockMetadata);
const actual = parameterizedQueryToSql(generatedSql);
expect(actual).toMatchSnapshot();
});
it('should render a chart config CTE configuration correctly', async () => {
const config: ChartConfigWithOptDateRange = {
connection: 'test-connection',
with: [
{
name: 'Parts',
sql: {
select: '_part, _part_offset',
from: { databaseName: 'default', tableName: 'some_table' },
where: '',
whereLanguage: 'sql',
filters: [
{
type: 'sql',
condition: `FieldA = 'test'`,
},
],
orderBy: [{ ordering: 'DESC', valueExpression: 'rand()' }],
limit: { limit: 1000 },
it('should render a chart config CTE configuration correctly', async () => {
const config: ChartConfigWithOptDateRange = {
connection: 'test-connection',
with: [
{
name: 'Parts',
chartConfig: {
connection: 'test-connection',
timestampValueExpression: '',
select: '_part, _part_offset',
from: { databaseName: 'default', tableName: 'some_table' },
where: '',
whereLanguage: 'sql',
filters: [
{
type: 'sql',
condition: `FieldA = 'test'`,
},
],
orderBy: [{ ordering: 'DESC', valueExpression: 'rand()' }],
limit: { limit: 1000 },
},
},
],
select: '*',
filters: [
{
type: 'sql',
condition: `FieldA = 'test'`,
},
{
type: 'sql',
condition: `indexHint((_part, _part_offset) IN (SELECT tuple(_part, _part_offset) FROM Parts))`,
},
],
from: {
databaseName: '',
tableName: 'Parts',
},
],
select: '*',
filters: [
{
type: 'sql',
condition: `FieldA = 'test'`,
},
{
type: 'sql',
condition: `indexHint((_part, _part_offset) IN (SELECT tuple(_part, _part_offset) FROM Parts))`,
},
],
from: {
databaseName: '',
tableName: 'Parts',
},
where: '',
whereLanguage: 'sql',
orderBy: [{ ordering: 'DESC', valueExpression: 'rand()' }],
limit: { limit: 1000 },
};
where: '',
whereLanguage: 'sql',
orderBy: [{ ordering: 'DESC', valueExpression: 'rand()' }],
limit: { limit: 1000 },
};
const generatedSql = await renderChartConfig(config, mockMetadata);
const actual = parameterizedQueryToSql(generatedSql);
expect(actual).toMatchSnapshot();
const generatedSql = await renderChartConfig(config, mockMetadata);
const actual = parameterizedQueryToSql(generatedSql);
expect(actual).toMatchSnapshot();
});
it('should throw if the CTE is missing both sql and chartConfig', async () => {
const config: ChartConfigWithOptDateRange = {
connection: 'test-connection',
with: [
{
name: 'InvalidCTE',
// Intentionally omitting both sql and chartConfig properties
},
],
select: [{ valueExpression: 'Line' }],
from: {
databaseName: 'default',
tableName: 'some_table',
},
where: '',
whereLanguage: 'sql',
};
await expect(renderChartConfig(config, mockMetadata)).rejects.toThrow(
"must specify either 'sql' or 'chartConfig' in with clause",
);
});
it('should throw if the CTE sql param is invalid', async () => {
const config: ChartConfigWithOptDateRange = {
connection: 'test-connection',
with: [
{
name: 'InvalidCTE',
sql: 'SELECT * FROM some_table' as any, // Intentionally not a ChSql object
},
],
select: [{ valueExpression: 'Line' }],
from: {
databaseName: 'default',
tableName: 'some_table',
},
where: '',
whereLanguage: 'sql',
};
await expect(renderChartConfig(config, mockMetadata)).rejects.toThrow(
'non-conforming sql object in CTE',
);
});
it('should throw if the CTE chartConfig param is invalid', async () => {
const config: ChartConfigWithOptDateRange = {
connection: 'test-connection',
with: [
{
name: 'InvalidCTE',
chartConfig: {
// Missing required properties like select, from, etc.
connection: 'test-connection',
} as any, // Intentionally invalid chartConfig
},
],
select: [{ valueExpression: 'Line' }],
from: {
databaseName: 'default',
tableName: 'some_table',
},
where: '',
whereLanguage: 'sql',
};
await expect(renderChartConfig(config, mockMetadata)).rejects.toThrow(
'non-conforming chartConfig object in CTE',
);
});
});
});

56
packages/common-utils/src/renderChartConfig.ts
View file

@ -7,9 +7,11 @@ import { CustomSchemaSQLSerializerV2, SearchQueryBuilder } from '@/queryParser';
import {
AggregateFunction,
AggregateFunctionWithCombinators,
ChartConfig,
ChartConfigSchema,
ChartConfigWithDateRange,
ChartConfigWithOptDateRange,
ChSqlSchema,
MetricsDataType,
SearchCondition,
SearchConditionLanguage,
@ -745,32 +747,42 @@ async function renderWith(
',',
await Promise.all(
withClauses.map(async clause => {
// The sql logic can be specified as either a string, ChSql instance or a
// chart config object.
let resolvedSql: ChSql;
if (typeof clause.sql === 'string') {
resolvedSql = chSql`${{ Identifier: clause.sql }}`;
} else if (clause.sql && 'sql' in clause.sql) {
resolvedSql = clause.sql;
} else if (
clause.sql &&
('select' in clause.sql || 'connection' in clause.sql)
) {
resolvedSql = await renderChartConfig(
{
...clause.sql,
connection: chartConfig.connection,
timestampValueExpression:
chartConfig.timestampValueExpression || '',
} as ChartConfigWithOptDateRangeEx,
metadata,
);
} else {
const {
sql,
chartConfig,
}: { sql?: ChSql; chartConfig?: ChartConfig } = clause;
// The sql logic can be specified as either a ChSql instance or a chart
// config object. Due to type erasure and the recursive nature of ChartConfig
// when using CTEs, we need to validate the types here to ensure junk did
// not make it through.
if (sql && chartConfig) {
throw new Error(
`ChartConfig with clause is an unsupported type: ${clause.sql}`,
"cannot specify both 'sql' and 'chartConfig' in with clause",
);
}
if (!(sql || chartConfig)) {
throw new Error(
"must specify either 'sql' or 'chartConfig' in with clause",
);
}
if (sql && !ChSqlSchema.safeParse(sql).success) {
throw new Error('non-conforming sql object in CTE');
}
if (
chartConfig &&
!ChartConfigSchema.safeParse(chartConfig).success
) {
throw new Error('non-conforming chartConfig object in CTE');
}
const resolvedSql = sql
? sql
: await renderChartConfig(chartConfig, metadata);
if (clause.isSubquery === false) {
return chSql`(${resolvedSql}) AS ${{ Identifier: clause.name }}`;
}

8
packages/common-utils/src/types.ts
View file

@ -125,7 +125,13 @@ export const SelectSQLStatementSchema = z.object({
.array(
z.object({
name: z.string(),
sql: z.lazy(() => ChSqlSchema.or(ChartConfigSchema)),
// Need to specify either a sql or chartConfig instance. To avoid
// the schema falling into an any type, the fields are separate
// and listed as optional.
sql: ChSqlSchema.optional(),
chartConfig: z.lazy(() => ChartConfigSchema).optional(),
// If true, it'll render as WITH ident AS (subquery)
// If false, it'll be a "variable" ex. WITH (sql) AS ident
// where sql can be any expression, ex. a constant string