Skip to content

Refactor/analytics#97

Merged
SteakFisher merged 15 commits into
ScrawnDotDev:devfrom
0xanshu:refactor/analytics
Jul 4, 2026
Merged

Refactor/analytics#97
SteakFisher merged 15 commits into
ScrawnDotDev:devfrom
0xanshu:refactor/analytics

Conversation

@0xanshu

@0xanshu 0xanshu commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

This pull request introduces a major refactor to event querying by implementing a dialect-based abstraction for querying event data from both Postgres and ClickHouse backends. The logic for building queries, filtering, and aggregations is now encapsulated in dialect classes, improving modularity and maintainability. The main query handler now delegates to these dialects, and the previous table/field/condition logic is removed from the gRPC route in favor of a unified, backend-agnostic approach.

Event Query Refactor and Dialect Abstraction:

Query Routing and Handler Abstraction

  • Introduced a new handler handleQueryEvents that routes event queries to the appropriate dialect (Postgres or ClickHouse) based on the configured storage adapter, and delegates all query and aggregation logic to the dialect classes. (src/storage/adapter/common/queryEvents.ts)

Dialect Implementations

  • Added PostgresQueryDialect and ClickHouseQueryDialect classes that encapsulate all logic for building, executing, and aggregating queries against their respective databases, including SQL generation, field mapping, and result normalization. (src/storage/adapter/common/postgresDialect.ts, src/storage/adapter/common/clickHouseDialect.ts)
  • Defined a common QueryDialect interface specifying the methods required for dialects, ensuring consistent usage and easier extensibility. (src/storage/adapter/common/sqlDialect.ts)

Query Route Refactor

  • Refactored the gRPC queryData route to remove all table/field/condition logic, and instead delegate query execution to a new executeDataQuery and getDataTable abstraction, aligning with the new dialect-based approach. (src/routes/gRPC/data/query.ts)

This refactor centralizes and standardizes event querying, making it easier to support multiple backends and reducing duplicate query logic across the codebase.

SteakFisher and others added 5 commits June 25, 2026 23:51
…dularization)

Phase 1 — SQL Dialect Abstraction:
- fieldRegistry.ts: single source of truth for event field definitions
- sqlDialect.ts: common interface (buildSelect, buildWhere, getTotalCount)
- postgresDialect.ts / clickHouseDialect.ts: dialect implementations
- queryEvents.ts: unified handler that picks dialect via STORAGE_ADAPTER
- Both PG and CH handlers now thin re-exports from common/queryEvents

Phase 3 — Data Engine Modularization:
- Extract dataQuery module from routes/gRPC/data/query into storage/query/
- Explicit field alias registry with auto-derived cast types from Drizzle metadata
- Thin handler in routes/gRPC/data/query delegates to executeDataQuery/getDataTable
- hasMore support in DataQueryResult
@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR refactors event querying into backend-specific dialects. The main changes are:

  • Adds shared Postgres and ClickHouse query dialects for event lists, counts, filters, and aggregations.
  • Routes queryEvents through the configured storage adapter and shared dialect handler.
  • Moves data table querying into reusable storage helpers with validation, auth scoping, pagination, and ordering.
  • Centralizes event field mappings and output aliases in a shared registry.
  • Adds gRPC tests for data queries, event filters, pagination, and aggregations.

Confidence Score: 4/5

Mostly safe to merge once the already-threaded review findings are resolved or confirmed addressed.

The current diff shows fixes for several earlier query issues, including tenant scoping, provider and output-cache mapping, metadata serialization, event type predicates, unknown aggregation validation, and ClickHouse pagination. No new distinct verified bug was found in this final pass. The score stays below the maximum because this is core query and auth-adjacent SQL code with backend-specific behavior.

src/storage/adapter/common/postgresDialect.ts, src/storage/adapter/common/clickHouseDialect.ts, src/storage/query/dataQuery.ts, src/storage/adapter/common/fieldRegistry.ts

T-Rex T-Rex Logs

What T-Rex did

  • Compared the before and after artifacts to verify that the new centralized dialect abstraction routes Postgres and ClickHouse queries as expected.
  • Verified the data-query-abstraction changes by inspecting the after-state log, confirming the metadata registry update and the expanded users schema.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
src/tests/dataQuery.test.ts Adds gRPC coverage for data table querying, filters, pagination, and current order-by behavior.
src/tests/queryEvents.test.ts Adds gRPC coverage for cross-backend event listing and aggregation across basic and AI usage events.
src/routes/gRPC/data/query.ts Simplifies the data query route to validate input, enforce dashboard auth, and delegate query execution.
src/routes/gRPC/query/queryEvents.ts Refactors query events response construction around the storage adapter result shapes.
src/storage/adapter/common/clickHouseDialect.ts Adds the ClickHouse dialect for list, aggregate, count, filtering, normalization, and global pagination.
src/storage/adapter/common/fieldRegistry.ts Centralizes backend-specific event field mappings, output aliases, and aggregate expressions.
src/storage/adapter/common/postgresDialect.ts Adds the Postgres dialect for union-backed event list and aggregation queries.
src/storage/query/dataQuery.ts Moves data table registry, auth scoping, filtering, ordering, counting, and pagination into a reusable helper.

Comments Outside Diff (1)

  1. General comment

    P1 Existing data query responses now expose projectId columns

    • Bug
      • For the same authenticated users query, base returned columns id,lastBilledTimestamp,paymentProviderUserId,mode, while head returned projectId,id,lastBilledTimestamp,paymentProviderUserId,mode. This changes the user-visible response contract for an existing table and includes the auth-scoping project identifier in every row. The validation request explicitly expected existing data query behavior and response shape to be preserved apart from the new metadata table behavior.
    • Cause
      • The new table registry in src/storage/query/dataQuery.ts adds projectId to the fields map for existing scoped tables. executeDataQuery uses the same fields map both for scoping (authConditions) and for buildSelect/columns, so scoping-only fields become visible result columns.
    • Fix
      • Separate scoping columns from public query fields, or add metadata on field definitions such as selectable: false for internal scoping fields. Use projectId for auth filters without including it in buildSelect, returned columns, or user-visible field validation unless intentionally documented as a response contract change.

    T-Rex Ran code and verified through T-Rex

Reviews (10): Last reviewed commit: "fix(analytics): reject unknown aggregati..." | Re-trigger Greptile

Comment thread src/storage/query/dataQuery.ts Outdated
Comment thread src/storage/adapter/common/queryEvents.ts
Comment thread src/storage/adapter/common/clickHouseDialect.ts Outdated
Comment thread src/storage/adapter/common/fieldRegistry.ts
Comment thread src/storage/adapter/common/postgresDialect.ts Outdated
Comment thread src/storage/adapter/common/clickHouseDialect.ts
Comment thread src/storage/adapter/common/fieldRegistry.ts
Comment thread src/storage/adapter/common/fieldRegistry.ts
Comment thread src/__tests__/queryEvents.test.ts Outdated
Comment thread src/storage/adapter/common/postgresDialect.ts
@SteakFisher

Copy link
Copy Markdown
Member

yea no way in fuck Im reviewing all that.. LGTM

@SteakFisher SteakFisher merged commit 462c9ff into ScrawnDotDev:dev Jul 4, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants