Skip to content

Commit 6936ef8

Browse files
authored
Optimize materialized column lookup for expression aliases (#1959)
## Summary Improve the materialized column optimization by allowing it to be applied when `WITH` clauses are expression aliases (i.e., `isSubquery: false`). Previously, any `WITH` clause would disable this optimization. This change ensures that materialized columns are still considered for performance benefits when the `WITH` clause does not represent a subquery. ### Screenshots or video | Before | After | | :----- | :---- | | | | ### How to test locally or on Vercel 1. Create a ClickHouse table with a materialized column, e.g.: ```sql ALTER TABLE otel_logs ADD COLUMN awesome_attribute String MATERIALIZED LogAttributes['awesome_attribute'] ``` 2. Open the Explore view for logs (`/search`) 3. Add a filter for `awesome_attribute` (or `LogAttributes['awesome_attribute']`) 4. Inspect the POST body of `/clickhouse-proxy` requests in the network tab: - Before fix: The histogram (time chart) query contains `LogAttributes['awesome_attribute']` (full map scan), while the search results query correctly uses `awesome_attribute`. - After fix: Both the histogram and search results queries use `awesome_attribute` (the materialized column). ### References - Linear Issue: #1957 - Related PRs:
1 parent 470b2c2 commit 6936ef8

7 files changed

Lines changed: 96 additions & 5 deletions

File tree

.changeset/wet-ads-shout.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@hyperdx/common-utils': patch
3+
---
4+
5+
fix: Enable materialized column optimization for expression alias CTEs

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ packages/app/next-env.d.ts
5151

5252
# optional npm cache directory
5353
**/.npm
54+
package-lock.json
5455

5556
# dependency directories
5657
**/node_modules

packages/app/tests/e2e/core/navigation.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ test.describe('Navigation', { tag: ['@core'] }, () => {
8888
});
8989
},
9090
);
91+
9192
test('should open user menu', async ({ page }) => {
9293
await test.step('Navigate to and click user menu trigger', async () => {
9394
// Wait for page to be fully loaded first

packages/app/tests/e2e/features/dashboard.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
6969
});
7070

7171
let dashboardUrl: string;
72+
7273
await test.step('Save dashboard URL', async () => {
7374
dashboardUrl = dashboardPage.page.url();
7475
console.log(`Dashboard URL: ${dashboardUrl}`);
@@ -113,8 +114,10 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
113114
await expect(dashboardTiles).toHaveCount(1);
114115
});
115116
});
117+
116118
test('Comprehensive dashboard workflow - create, add tiles, configure, and test', async () => {
117119
test.setTimeout(60000);
120+
118121
await test.step('Create new dashboard', async () => {
119122
await expect(dashboardPage.createButton).toBeVisible();
120123
await dashboardPage.createNewDashboard();
@@ -347,6 +350,7 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
347350
});
348351

349352
let dashboardUrl: string;
353+
350354
await test.step('Save dashboard URL', async () => {
351355
dashboardUrl = dashboardPage.page.url();
352356
console.log(`Dashboard URL: ${dashboardUrl}`);

packages/app/tests/e2e/features/search/saved-search.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,7 @@ test.describe('Saved Search Functionality', () => {
431431

432432
let savedSearchUrl: string;
433433
let appliedFilterValue: string;
434+
434435
await test.step('Apply filters in the sidebar', async () => {
435436
const [picked] = await searchPage.filters.pickVisibleFilterValues(
436437
'SeverityText',

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,78 @@ describe('renderChartConfig', () => {
590590
});
591591
});
592592

593+
describe('materialized column optimization with expression alias CTEs', () => {
594+
it('should rewrite WHERE to use materialized column when with clauses are expression aliases (isSubquery: false)', async () => {
595+
mockMetadata.getMaterializedColumnsLookupTable = jest
596+
.fn()
597+
.mockResolvedValue(
598+
new Map([["LogAttributes['attr_key']", 'attr_key']]),
599+
);
600+
601+
const config: ChartConfigWithOptDateRange = {
602+
connection: 'test-connection',
603+
from: {
604+
databaseName: 'default',
605+
tableName: 'otel_logs',
606+
},
607+
with: [
608+
{
609+
name: 'body',
610+
sql: chSql`toString(Body)`,
611+
isSubquery: false,
612+
},
613+
],
614+
select: [{ aggFn: 'count', valueExpression: '' }],
615+
where: "LogAttributes['attr_key'] = 'attr_val'",
616+
whereLanguage: 'sql',
617+
granularity: '1 minute',
618+
timestampValueExpression: 'Timestamp',
619+
dateRange: [new Date('2025-01-01'), new Date('2025-01-02')],
620+
};
621+
622+
const generatedSql = await renderChartConfig(
623+
config,
624+
mockMetadata,
625+
querySettings,
626+
);
627+
const sql = parameterizedQueryToSql(generatedSql);
628+
629+
expect(mockMetadata.getMaterializedColumnsLookupTable).toHaveBeenCalled();
630+
expect(sql).toContain("attr_key = 'attr_val'");
631+
expect(sql).not.toContain("LogAttributes['attr_key']");
632+
});
633+
634+
it('should skip materialized columns when with clauses are subquery CTEs', async () => {
635+
mockMetadata.getMaterializedColumnsLookupTable = jest
636+
.fn()
637+
.mockResolvedValue(
638+
new Map([["LogAttributes['attr_key']", 'attr_key']]),
639+
);
640+
641+
const config: ChartConfigWithOptDateRange = {
642+
connection: 'test-connection',
643+
from: {
644+
databaseName: '',
645+
tableName: 'TestCte',
646+
},
647+
with: [
648+
{
649+
name: 'TestCte',
650+
sql: chSql`SELECT * FROM otel_logs`,
651+
},
652+
],
653+
select: [{ aggFn: 'count', valueExpression: '' }],
654+
where: '',
655+
whereLanguage: 'sql',
656+
};
657+
658+
await renderChartConfig(config, mockMetadata, querySettings);
659+
expect(
660+
mockMetadata.getMaterializedColumnsLookupTable,
661+
).not.toHaveBeenCalled();
662+
});
663+
});
664+
593665
describe('k8s semantic convention migrations', () => {
594666
it('should generate SQL with metricNameSql for k8s.pod.cpu.utilization gauge metric', async () => {
595667
const config: ChartConfigWithOptDateRange = {

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,12 @@ export function isNonEmptyWhereExpr(where?: string): where is string {
180180
return where != null && where.trim() != '';
181181
}
182182

183+
function hasSubqueryCte(
184+
withClauses: BuilderChartConfigWithDateRange['with'],
185+
): boolean {
186+
return withClauses?.some(w => w.isSubquery !== false) ?? false;
187+
}
188+
183189
const fastifySQL = ({
184190
materializedFields,
185191
rawSQL,
@@ -421,13 +427,14 @@ async function renderSelectList(
421427

422428
// This metadata query is executed in an attempt tp optimize the selects by favoring materialized fields
423429
// on a view/table that already perform the computation in select. This optimization is not currently
424-
// supported for queries using CTEs so skip the metadata fetch if there are CTE objects in the config.
430+
// supported for queries using subquery CTEs so skip the metadata fetch if there are subquery CTE
431+
// objects in the config. Expression aliases (isSubquery: false) do not affect the base table.
425432
let materializedFields: Map<string, string> | undefined;
426433
try {
427434
// This will likely error when referencing a CTE, which is assumed
428435
// to be the case when chartConfig.from.databaseName is not set.
429436
materializedFields =
430-
chartConfig.with?.length || !chartConfig.from.databaseName
437+
hasSubqueryCte(chartConfig.with) || !chartConfig.from.databaseName
431438
? undefined
432439
: await metadata.getMaterializedColumnsLookupTable({
433440
connectionId: chartConfig.connection,
@@ -726,14 +733,14 @@ async function renderWhereExpressionStr({
726733

727734
// This metadata query is executed in an attempt tp optimize the selects by favoring materialized fields
728735
// on a view/table that already perform the computation in select. This optimization is not currently
729-
// supported for queries using CTEs so skip the metadata fetch if there are CTE objects in the config.
730-
736+
// supported for queries using subquery CTEs so skip the metadata fetch if there are subquery CTE
737+
// objects in the config. Expression aliases (isSubquery: false) do not affect the base table.
731738
let materializedFields: Map<string, string> | undefined;
732739
try {
733740
// This will likely error when referencing a CTE, which is assumed
734741
// to be the case when from.databaseName is not set.
735742
materializedFields =
736-
withClauses?.length || !from.databaseName
743+
hasSubqueryCte(withClauses) || !from.databaseName
737744
? undefined
738745
: await metadata.getMaterializedColumnsLookupTable({
739746
connectionId,

0 commit comments

Comments
 (0)