Skip to content

Optimize materialized column lookup for expression aliases#1959

Merged
kodiakhq[bot] merged 3 commits intohyperdxio:mainfrom
vinzee:va/fix_materialized_column_optimization_for_histograms
Mar 22, 2026
Merged

Optimize materialized column lookup for expression aliases#1959
kodiakhq[bot] merged 3 commits intohyperdxio:mainfrom
vinzee:va/fix_materialized_column_optimization_for_histograms

Conversation

@vinzee
Copy link
Copy Markdown
Contributor

@vinzee vinzee commented Mar 21, 2026

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.:
    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

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.
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 21, 2026

@vinzee is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 21, 2026

🦋 Changeset detected

Latest commit: 677b768

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hyperdx/common-utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 21, 2026

PR Review

✅ No critical issues found.

The core change is minimal and correct:

  • hasSubqueryCte() properly handles the optional isSubquery field — since it defaults to true (per the type comment "default assume true"), w.isSubquery !== false correctly treats undefined as a subquery CTE and only allows the optimization when explicitly false.
  • Both renderSelectList and renderWhereExpressionStr are updated consistently.
  • Tests cover both the new path (expression alias CTEs → optimization enabled) and the existing subquery CTE path (optimization disabled).
  • Changeset is included.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Mar 22, 2026 9:05pm

Request Review

Copy link
Copy Markdown
Member

@wrn14897 wrn14897 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run yarn changeset to add a changeset

await renderChartConfig(config, mockMetadata, querySettings);
expect(
mockMetadata.getMaterializedColumnsLookupTable,
).toHaveBeenCalled();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to verify the rendered SQL has pipe_id

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed ✅

// 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.
const hasSubqueryCte = chartConfig.with?.some(w => w.isSubquery !== false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we move this logic to a shared method?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done ✅

Extract a shared hasSubqueryCte() helper that only skips the optimization for true
subquery CTEs, and fix unit tests verifying both paths.
wrn14897
wrn14897 previously approved these changes Mar 22, 2026
Copy link
Copy Markdown
Member

@wrn14897 wrn14897 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@wrn14897
Copy link
Copy Markdown
Member

@claude fix linting error

@github-actions
Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@wrn14897
Copy link
Copy Markdown
Member

@vinzee there is a lint error in the test file. could you fix it? https://github.com/hyperdxio/hyperdx/actions/runs/23395356725/job/68092930851?pr=1959

This commit refactors several E2E tests by adding blank lines for improved readability and consistency. It also updates the .gitignore file to ignore `package-lock.json`, preventing it from being committed to the repository.

[Generated via Zoox Intelligence / `zi-gcm --edit`]
@kodiakhq kodiakhq Bot merged commit 6936ef8 into hyperdxio:main Mar 22, 2026
15 checks passed
knudtty pushed a commit that referenced this pull request Apr 16, 2026
## 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:
Copilot AI pushed a commit that referenced this pull request Apr 20, 2026
## 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:
Co-authored-by: peter-leonov-ch <209667683+peter-leonov-ch@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants