Skip to content

Commit 5d78984

Browse files
authored
fix cutom events charts (#134)
1 parent c95c9aa commit 5d78984

4 files changed

Lines changed: 58 additions & 14 deletions

File tree

packages/db/src/services/chart.service.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -600,11 +600,14 @@ export async function getChartSql({
600600
return buildInlineCohortJoin(cohortId, projectId, 'e', cohortMeta);
601601
}).join(' ');
602602

603+
// Determine data source: use custom event CTE if present, otherwise events table
604+
const dataSource = customEvent ? 'custom_event_data' : TABLE_NAMES.events;
605+
603606
// Add top_breakdowns CTE using the builder
604607
addCte(
605608
'top_breakdowns',
606609
`SELECT ${breakdownSelects}
607-
FROM ${TABLE_NAMES.events} e
610+
FROM ${dataSource} AS e
608611
${profilesJoinRef ? `${profilesJoinRef} ` : ''}${cohortJoinsForTop ? `${cohortJoinsForTop} ` : ''}${getWhereWithoutBar()}
609612
GROUP BY ${breakdownSelects}
610613
ORDER BY count(*) DESC
@@ -685,19 +688,22 @@ export async function getChartSql({
685688

686689
const totalCountWhere = getWhereWithoutBar();
687690

691+
// Determine data source: use custom event CTE if present, otherwise events table
692+
const dataSourceForBreakdown = customEvent ? 'custom_event_data' : TABLE_NAMES.events;
693+
688694
// Build cohort JOINs for breakdown_totals CTE
689695
// NOTE: ClickHouse CTEs cannot reference other CTEs in JOINs, so we inline the subquery
690696
const cohortJoinsForBreakdown = cohortIds.map((cohortId) => {
691697
const cohortMeta = cohortMetadata.get(cohortId);
692-
return buildInlineCohortJoin(cohortId, projectId, TABLE_NAMES.events, cohortMeta);
698+
return buildInlineCohortJoin(cohortId, projectId, dataSourceForBreakdown, cohortMeta);
693699
}).join(' ');
694700

695701
addCte(
696702
'breakdown_totals',
697703
`SELECT
698704
${breakdownSelects},
699-
uniq(${TABLE_NAMES.events}.profile_id) as total_count
700-
FROM ${TABLE_NAMES.events}
705+
uniq(${dataSourceForBreakdown}.profile_id) as total_count
706+
FROM ${dataSourceForBreakdown}
701707
${profilesJoinRefForCTE ? `${profilesJoinRefForCTE} ` : ''}${cohortJoinsForBreakdown ? `${cohortJoinsForBreakdown} ` : ''}${totalCountWhere}
702708
GROUP BY ${breakdownGroupBy}`,
703709
);
@@ -714,17 +720,20 @@ export async function getChartSql({
714720
} else {
715721
const totalCountWhere = getWhereWithoutBar();
716722

723+
// Determine data source: use custom event CTE if present, otherwise events table
724+
const dataSourceForTotal = customEvent ? 'custom_event_data' : TABLE_NAMES.events;
725+
717726
// Build cohort JOINs for total_unique CTE
718727
// NOTE: ClickHouse CTEs cannot reference other CTEs in JOINs, so we inline the subquery
719728
const cohortJoinsForTotal = cohortIds.map((cohortId) => {
720729
const cohortMeta = cohortMetadata.get(cohortId);
721-
return buildInlineCohortJoin(cohortId, projectId, TABLE_NAMES.events, cohortMeta);
730+
return buildInlineCohortJoin(cohortId, projectId, dataSourceForTotal, cohortMeta);
722731
}).join(' ');
723732

724733
addCte(
725734
'total_unique',
726-
`SELECT uniq(${TABLE_NAMES.events}.profile_id) as total_count
727-
FROM ${TABLE_NAMES.events}
735+
`SELECT uniq(${dataSourceForTotal}.profile_id) as total_count
736+
FROM ${dataSourceForTotal}
728737
${profilesJoinRefForCTE ? `${profilesJoinRefForCTE} ` : ''}${cohortJoinsForTotal ? `${cohortJoinsForTotal} ` : ''}${totalCountWhere}`,
729738
);
730739

packages/db/src/services/conversion.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ export class ConversionService {
159159
// Build funnel conditions for all events
160160
const conditions = events.map(event => {
161161
const where = Object.values(
162-
getEventFiltersWhereClause(event.filters),
162+
getEventFiltersWhereClause(event.filters, projectId),
163163
).join(' AND ');
164164

165165
return where

packages/db/src/services/custom-event.service.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,17 @@ function buildCustomEventSourceQuery(
6464
}
6565

6666
/**
67-
* Expand a custom event into SQL UNION of source events
68-
* This creates a CTE that can be used in place of the events table
67+
* Expand a custom event into SQL query
68+
*
69+
* PERFORMANCE OPTIMIZATION:
70+
* When all source events have no filters, we use a single SELECT with IN clause
71+
* instead of UNION ALL. This reduces N table scans to 1 table scan.
72+
*
73+
* Example:
74+
* - Before: 20 separate SELECTs with UNION ALL (20 table scans)
75+
* - After: 1 SELECT with IN ('event1', 'event2', ..., 'event20') (1 table scan)
76+
*
77+
* For events with filters, we fall back to UNION ALL to ensure correct filtering.
6978
*
7079
* @param customEvent - The custom event definition
7180
* @param baseWhere - Base WHERE conditions to apply to all source events (date ranges, etc)
@@ -81,6 +90,33 @@ export function expandCustomEventToSQL(
8190
): string {
8291
const definition = customEvent.definition;
8392

93+
// Check if we can use the optimized path
94+
// Optimization is only safe when all events have no filters
95+
const canOptimize = definition.events.every(
96+
(event) => !event.filters || event.filters.length === 0
97+
);
98+
99+
if (canOptimize && definition.events.length > 0) {
100+
// OPTIMIZED PATH: Single SELECT with IN clause
101+
// This reduces N table scans to 1 table scan, dramatically improving performance
102+
const eventNames = definition.events.map((e) => sqlstring.escape(e.name));
103+
104+
const whereClauses = [
105+
`project_id = ${sqlstring.escape(customEvent.projectId)}`,
106+
`name IN (${eventNames.join(', ')})`,
107+
...baseWhere,
108+
];
109+
110+
return `
111+
SELECT * REPLACE(${sqlstring.escape(customEvent.name)} AS name)
112+
FROM ${TABLE_NAMES.events}
113+
WHERE ${whereClauses.join(' AND ')}
114+
`;
115+
}
116+
117+
// FALLBACK PATH: UNION ALL for events with filters
118+
// When events have filters, each event may need different WHERE conditions,
119+
// so we must use UNION ALL to apply filters correctly per event
84120
const sourceQueries = definition.events.map((sourceEvent) =>
85121
buildCustomEventSourceQuery(
86122
customEvent.projectId,
@@ -90,6 +126,5 @@ export function expandCustomEventToSQL(
90126
),
91127
);
92128

93-
// UNION ALL for OR logic (match any source event)
94129
return sourceQueries.join(' UNION ALL ');
95130
}

packages/db/src/services/funnel.service.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ export class FunnelService {
113113
};
114114
}
115115

116-
getFunnelConditions(events: IChartEvent[] = []): string[] {
116+
getFunnelConditions(events: IChartEvent[] = [], projectId?: string): string[] {
117117
return events.map((event) => {
118118
const { sb, getWhere } = createSqlBuilder();
119-
sb.where = getEventFiltersWhereClause(event.filters);
119+
sb.where = getEventFiltersWhereClause(event.filters, projectId);
120120
sb.where.name = `name = ${sqlstring.escape(event.name)}`;
121121
return getWhere().replace('WHERE ', '');
122122
});
@@ -147,7 +147,7 @@ export class FunnelService {
147147
fromClause: string;
148148
needsNameFilter: boolean;
149149
}) {
150-
const funnels = this.getFunnelConditions(eventSeries);
150+
const funnels = this.getFunnelConditions(eventSeries, projectId);
151151

152152
const query = clix(this.client, timezone)
153153
.select([

0 commit comments

Comments
 (0)