mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
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:
This commit is contained in:
parent
470b2c2992
commit
6936ef8e29
7 changed files with 96 additions and 5 deletions
5
.changeset/wet-ads-shout.md
Normal file
5
.changeset/wet-ads-shout.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
'@hyperdx/common-utils': patch
|
||||
---
|
||||
|
||||
fix: Enable materialized column optimization for expression alias CTEs
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
|
|
@ -51,6 +51,7 @@ packages/app/next-env.d.ts
|
|||
|
||||
# optional npm cache directory
|
||||
**/.npm
|
||||
package-lock.json
|
||||
|
||||
# dependency directories
|
||||
**/node_modules
|
||||
|
|
|
|||
|
|
@ -88,6 +88,7 @@ test.describe('Navigation', { tag: ['@core'] }, () => {
|
|||
});
|
||||
},
|
||||
);
|
||||
|
||||
test('should open user menu', async ({ page }) => {
|
||||
await test.step('Navigate to and click user menu trigger', async () => {
|
||||
// Wait for page to be fully loaded first
|
||||
|
|
|
|||
|
|
@ -69,6 +69,7 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
|
|||
});
|
||||
|
||||
let dashboardUrl: string;
|
||||
|
||||
await test.step('Save dashboard URL', async () => {
|
||||
dashboardUrl = dashboardPage.page.url();
|
||||
console.log(`Dashboard URL: ${dashboardUrl}`);
|
||||
|
|
@ -113,8 +114,10 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
|
|||
await expect(dashboardTiles).toHaveCount(1);
|
||||
});
|
||||
});
|
||||
|
||||
test('Comprehensive dashboard workflow - create, add tiles, configure, and test', async () => {
|
||||
test.setTimeout(60000);
|
||||
|
||||
await test.step('Create new dashboard', async () => {
|
||||
await expect(dashboardPage.createButton).toBeVisible();
|
||||
await dashboardPage.createNewDashboard();
|
||||
|
|
@ -347,6 +350,7 @@ test.describe('Dashboard', { tag: ['@dashboard'] }, () => {
|
|||
});
|
||||
|
||||
let dashboardUrl: string;
|
||||
|
||||
await test.step('Save dashboard URL', async () => {
|
||||
dashboardUrl = dashboardPage.page.url();
|
||||
console.log(`Dashboard URL: ${dashboardUrl}`);
|
||||
|
|
|
|||
|
|
@ -431,6 +431,7 @@ test.describe('Saved Search Functionality', () => {
|
|||
|
||||
let savedSearchUrl: string;
|
||||
let appliedFilterValue: string;
|
||||
|
||||
await test.step('Apply filters in the sidebar', async () => {
|
||||
const [picked] = await searchPage.filters.pickVisibleFilterValues(
|
||||
'SeverityText',
|
||||
|
|
|
|||
|
|
@ -590,6 +590,78 @@ describe('renderChartConfig', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('materialized column optimization with expression alias CTEs', () => {
|
||||
it('should rewrite WHERE to use materialized column when with clauses are expression aliases (isSubquery: false)', async () => {
|
||||
mockMetadata.getMaterializedColumnsLookupTable = jest
|
||||
.fn()
|
||||
.mockResolvedValue(
|
||||
new Map([["LogAttributes['attr_key']", 'attr_key']]),
|
||||
);
|
||||
|
||||
const config: ChartConfigWithOptDateRange = {
|
||||
connection: 'test-connection',
|
||||
from: {
|
||||
databaseName: 'default',
|
||||
tableName: 'otel_logs',
|
||||
},
|
||||
with: [
|
||||
{
|
||||
name: 'body',
|
||||
sql: chSql`toString(Body)`,
|
||||
isSubquery: false,
|
||||
},
|
||||
],
|
||||
select: [{ aggFn: 'count', valueExpression: '' }],
|
||||
where: "LogAttributes['attr_key'] = 'attr_val'",
|
||||
whereLanguage: 'sql',
|
||||
granularity: '1 minute',
|
||||
timestampValueExpression: 'Timestamp',
|
||||
dateRange: [new Date('2025-01-01'), new Date('2025-01-02')],
|
||||
};
|
||||
|
||||
const generatedSql = await renderChartConfig(
|
||||
config,
|
||||
mockMetadata,
|
||||
querySettings,
|
||||
);
|
||||
const sql = parameterizedQueryToSql(generatedSql);
|
||||
|
||||
expect(mockMetadata.getMaterializedColumnsLookupTable).toHaveBeenCalled();
|
||||
expect(sql).toContain("attr_key = 'attr_val'");
|
||||
expect(sql).not.toContain("LogAttributes['attr_key']");
|
||||
});
|
||||
|
||||
it('should skip materialized columns when with clauses are subquery CTEs', async () => {
|
||||
mockMetadata.getMaterializedColumnsLookupTable = jest
|
||||
.fn()
|
||||
.mockResolvedValue(
|
||||
new Map([["LogAttributes['attr_key']", 'attr_key']]),
|
||||
);
|
||||
|
||||
const config: ChartConfigWithOptDateRange = {
|
||||
connection: 'test-connection',
|
||||
from: {
|
||||
databaseName: '',
|
||||
tableName: 'TestCte',
|
||||
},
|
||||
with: [
|
||||
{
|
||||
name: 'TestCte',
|
||||
sql: chSql`SELECT * FROM otel_logs`,
|
||||
},
|
||||
],
|
||||
select: [{ aggFn: 'count', valueExpression: '' }],
|
||||
where: '',
|
||||
whereLanguage: 'sql',
|
||||
};
|
||||
|
||||
await renderChartConfig(config, mockMetadata, querySettings);
|
||||
expect(
|
||||
mockMetadata.getMaterializedColumnsLookupTable,
|
||||
).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('k8s semantic convention migrations', () => {
|
||||
it('should generate SQL with metricNameSql for k8s.pod.cpu.utilization gauge metric', async () => {
|
||||
const config: ChartConfigWithOptDateRange = {
|
||||
|
|
|
|||
|
|
@ -180,6 +180,12 @@ export function isNonEmptyWhereExpr(where?: string): where is string {
|
|||
return where != null && where.trim() != '';
|
||||
}
|
||||
|
||||
function hasSubqueryCte(
|
||||
withClauses: BuilderChartConfigWithDateRange['with'],
|
||||
): boolean {
|
||||
return withClauses?.some(w => w.isSubquery !== false) ?? false;
|
||||
}
|
||||
|
||||
const fastifySQL = ({
|
||||
materializedFields,
|
||||
rawSQL,
|
||||
|
|
@ -421,13 +427,14 @@ async function renderSelectList(
|
|||
|
||||
// This metadata query is executed in an attempt tp optimize the selects by favoring materialized fields
|
||||
// on a view/table that already perform the computation in select. This optimization is not currently
|
||||
// supported for queries using CTEs so skip the metadata fetch if there are CTE objects in the config.
|
||||
// supported for queries using subquery CTEs so skip the metadata fetch if there are subquery CTE
|
||||
// objects in the config. Expression aliases (isSubquery: false) do not affect the base table.
|
||||
let materializedFields: Map<string, string> | undefined;
|
||||
try {
|
||||
// This will likely error when referencing a CTE, which is assumed
|
||||
// to be the case when chartConfig.from.databaseName is not set.
|
||||
materializedFields =
|
||||
chartConfig.with?.length || !chartConfig.from.databaseName
|
||||
hasSubqueryCte(chartConfig.with) || !chartConfig.from.databaseName
|
||||
? undefined
|
||||
: await metadata.getMaterializedColumnsLookupTable({
|
||||
connectionId: chartConfig.connection,
|
||||
|
|
@ -726,14 +733,14 @@ async function renderWhereExpressionStr({
|
|||
|
||||
// This metadata query is executed in an attempt tp optimize the selects by favoring materialized fields
|
||||
// on a view/table that already perform the computation in select. This optimization is not currently
|
||||
// supported for queries using CTEs so skip the metadata fetch if there are CTE objects in the config.
|
||||
|
||||
// supported for queries using subquery CTEs so skip the metadata fetch if there are subquery CTE
|
||||
// objects in the config. Expression aliases (isSubquery: false) do not affect the base table.
|
||||
let materializedFields: Map<string, string> | undefined;
|
||||
try {
|
||||
// This will likely error when referencing a CTE, which is assumed
|
||||
// to be the case when from.databaseName is not set.
|
||||
materializedFields =
|
||||
withClauses?.length || !from.databaseName
|
||||
hasSubqueryCte(withClauses) || !from.databaseName
|
||||
? undefined
|
||||
: await metadata.getMaterializedColumnsLookupTable({
|
||||
connectionId,
|
||||
|
|
|
|||
Loading…
Reference in a new issue