refactor(hogql): separate query and queryHogQL functions #32611
Conversation
There was a problem hiding this comment.
PR Summary
This PR introduces a new api.queryHogQL utility function that replaces direct HogQLQuery node construction across multiple components, aiming to centralize version management and improve type safety for HogQL queries.
- Added branded
HogQLQueryStringtype with template literal syntax for better type safety - Modified
HogQLQueryResponseinterface to support generic type parameter for flexible response types - Refactored query execution in components like
authorizedUrlListLogic,metalyticsLogic, andversionCheckerLogicto use the new utility - Removed explicit
NodeKind.HogQLQuerynode construction in favor of the new template literal approach - Maintained existing query functionality while making implementation more concise and maintainable
14 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
| const query = hogql` | ||
| SELECT distinct properties.$current_url AS urls | ||
| FROM events | ||
| WHERE timestamp >= now() - INTERVAL 7 DAY | ||
| AND timestamp <= now() | ||
| AND properties.$current_url like '%${hogql.identifier(values.browserSearchTerm)}%' | ||
| ORDER BY timestamp DESC | ||
| LIMIT 100` | ||
|
|
There was a problem hiding this comment.
style: SQL query indentation is inconsistent with the second query 8000 (lines 119-129). Consider standardizing the indentation style across all HogQL queries.
frontend/src/scenes/session-recordings/player/sessionRecordingDataLogic.ts
Outdated
Show resolved
Hide resolved
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
PR Summary
Additional changes and improvements to the HogQL query refactor implementation, focusing on type safety and template literal handling.
- Added error handling for type casting in
sessionRecordingDataLogic.tsfor concatenated queries toHogQLQueryStringtype - Introduced query response type parameter
Tinschema-general.tsfor more precise typing of query results - Simplified query interface by dropping
.query()in favor of direct.queryHogQL()in survey and heatmap logic components - Streamlined query versioning by centralizing version field application across all HogQL nodes
- Improved maintainability by using consistent hogql template literals for better type inference
14 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated3 snapshot changes in total. 0 added, 3 modified, 0 deleted:
Triggered by this commit. |
|
Size Change: +375 B (+0.02%) Total Size: 1.83 MB ℹ️ View Unchanged
|
Problem
I want to find all hard coded query nodes in our application for applying the latest
versionfield to them with a helper function. Turns out that most of our calls toapi.queryare actuallyHogQLQuerynodes and we can version them in a central place.Changes
api.queryHogQLutil that takes a brandedhogqltemplate literal.api.queryandapi.queryHogQL.Before:
await api.query(query, undefined, undefined, 'force_blocking')After:
await api.queryHogQL(query, { refresh: 'force_blocking' })How did you test this code?
CI run