Skip to content

Commit 3e92670

Browse files
harshitsaamuharshitsaamu
authored
fix conversion numbers (#140)
1 parent 17e50d0 commit 3e92670

1 file changed

Lines changed: 168 additions & 69 deletions

File tree

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

Lines changed: 168 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,98 @@ import { getCustomEventByName, expandCustomEventToSQL } from './custom-event.ser
1818
export class ConversionService {
1919
constructor(private client: typeof ch) {}
2020

21+
/**
22+
* Helper to build breakdown column with table alias
23+
* Handles property keys, profile fields, and cohort expressions
24+
*/
25+
private getBreakdownColumnWithAlias(
26+
breakdownName: string,
27+
projectId: string,
28+
cohortId: string | undefined,
29+
tableAlias: string,
30+
): string {
31+
const propertyKey = getSelectPropertyKey(breakdownName, projectId, cohortId);
32+
33+
// Cohort expressions already have their own aliases (e.g., cohort_abc123.profile_id)
34+
if (propertyKey.includes('cohort_') || propertyKey.startsWith('if(')) {
35+
return propertyKey;
36+
}
37+
38+
// Profile fields are already qualified (e.g., profile.created_at)
39+
if (propertyKey.startsWith('profile.')) {
40+
return propertyKey;
41+
}
42+
43+
// For property fields, prepend table alias
44+
// e.g., properties['key'] -> se.properties['key']
45+
if (propertyKey.startsWith('properties[') || propertyKey.includes('arrayMap')) {
46+
return `${tableAlias}.${propertyKey}`;
47+
}
48+
49+
// For simple fields (e.g., name, country), prepend alias
50+
return `${tableAlias}.${propertyKey}`;
51+
}
52+
53+
/**
54+
* Build CTE for a single event (start or end of funnel)
55+
* Handles both regular events and custom events
56+
*/
57+
private async buildSingleEventCte(
58+
event: IChartEvent,
59+
cteName: string,
60+
projectId: string,
61+
startDate: string,
62+
endDate: string,
63+
): Promise<string> {
64+
// Get materialized columns to ensure compatibility
65+
const materializedColumns = await getMaterializedColumns();
66+
const materializedColumnNames = Object.values(materializedColumns);
67+
const materializedColumnsSelect = materializedColumnNames.length > 0
68+
? `, ${materializedColumnNames.join(', ')}`
69+
: '';
70+
71+
// Check if this is a custom event
72+
const customEvent = await getCustomEventByName(event.name, projectId);
73+
74+
if (customEvent) {
75+
// Custom event - expand to SQL
76+
const baseWhere = [
77+
`created_at >= toDateTime('${formatClickhouseDate(startDate)}')`,
78+
`created_at <= toDateTime('${formatClickhouseDate(endDate)}')`,
79+
];
80+
81+
const sql = await expandCustomEventToSQL(
82+
{
83+
name: customEvent.name,
84+
projectId,
85+
definition: customEvent.definition as any,
86+
},
87+
baseWhere,
88+
);
89+
90+
return `${cteName} AS (${sql})`;
91+
} else {
92+
// Regular event - apply filters if present
93+
const filterWhere = event.filters && event.filters.length > 0
94+
? ' AND ' + Object.values(getEventFiltersWhereClause(event.filters, projectId)).join(' AND ')
95+
: '';
96+
97+
return `${cteName} AS (
98+
SELECT *${materializedColumnsSelect}
99+
FROM ${TABLE_NAMES.events}
100+
WHERE project_id = '${projectId}'
101+
AND name = '${event.name}'
102+
AND created_at >= toDateTime('${formatClickhouseDate(startDate)}')
103+
AND created_at <= toDateTime('${formatClickhouseDate(endDate)}')${filterWhere}
104+
)`;
105+
}
106+
}
107+
21108
/**
22109
* Build events source for conversion query
23110
* Handles both regular events and custom events
24111
* Supports N events (not just 2)
112+
* @deprecated Use buildSingleEventCte instead for optimized self-join approach
25113
*/
26114
private async buildEventsSource(
27115
events: IChartEvent[],
@@ -155,41 +243,37 @@ export class ConversionService {
155243

156244
const funnelWindowSeconds = funnelWindow * 3600;
157245

158-
// Get events source (handles custom events)
159-
const { fromClause, ctes, needsDateFilter } = await this.buildEventsSource(
160-
events,
246+
// Use first and last events for conversion tracking
247+
const firstEvent = events[0]!;
248+
const lastEvent = events[events.length - 1]!;
249+
250+
// Calculate extended end date for conversion events (add funnel window)
251+
const extendedEndDate = DateTime.fromISO(endDate)
252+
.plus({ seconds: funnelWindowSeconds })
253+
.toFormat('yyyy-MM-dd HH:mm:ss');
254+
255+
// Build CTEs for start and end events
256+
const ctes: string[] = [];
257+
258+
// Start events CTE (first event in funnel)
259+
const startEventCte = await this.buildSingleEventCte(
260+
firstEvent,
261+
'start_events',
161262
projectId,
162263
startDate,
163264
endDate,
164265
);
266+
ctes.push(startEventCte);
165267

166-
// Define group and breakdowns after fromClause is available
167-
const group = funnelGroup === 'profile_id' ? `${fromClause}.profile_id` : `${fromClause}.session_id`;
168-
const breakdownColumns = breakdowns.map(
169-
(b, index) => `${getSelectPropertyKey(b.name, projectId, b.cohortId)} as b_${index}`,
268+
// End events CTE (last event in funnel) - with extended date range
269+
const endEventCte = await this.buildSingleEventCte(
270+
lastEvent,
271+
'end_events',
272+
projectId,
273+
startDate,
274+
extendedEndDate,
170275
);
171-
const breakdownGroupBy = breakdowns.map((b, index) => `b_${index}`);
172-
173-
// Build funnel conditions for all events
174-
const conditions = events.map(event => {
175-
const where = Object.values(
176-
getEventFiltersWhereClause(event.filters, projectId),
177-
).join(' AND ');
178-
179-
return where
180-
? `(name = '${event.name}' AND ${where})`
181-
: `name = '${event.name}'`;
182-
});
183-
184-
// Build WHERE clause
185-
const whereClauses = [`project_id = '${projectId}'`];
186-
if (needsDateFilter) {
187-
whereClauses.push(
188-
`created_at BETWEEN toDateTime('${startDate}') AND toDateTime('${endDate}')`,
189-
);
190-
const eventNames = events.map(e => `'${e.name}'`).join(', ');
191-
whereClauses.push(`name IN (${eventNames})`);
192-
}
276+
ctes.push(endEventCte);
193277

194278
// Add cohort CTEs (computed once per query, not per row)
195279
cohortIds.forEach((cohortId) => {
@@ -198,22 +282,27 @@ export class ConversionService {
198282
ctes.push(`${getCohortCteName(cohortId)} AS (${cohortQuery})`);
199283
});
200284

201-
// Build WITH clause if CTEs exist
202-
const withClause = ctes.length > 0 ? `WITH ${ctes.join(', ')} ` : '';
285+
// Define group column (profile_id or session_id)
286+
const groupCol = funnelGroup === 'profile_id' ? 'profile_id' : 'session_id';
203287

204-
// Build LEFT JOINs for all cohorts (much faster than IN subqueries)
205-
const cohortJoins = cohortIds.length > 0 ? '\n ' + cohortIds.map((cohortId) => {
288+
// Build breakdown columns (from start_events with 'se' alias)
289+
const breakdownColumns = breakdowns.map((b, index) => {
290+
const columnWithAlias = this.getBreakdownColumnWithAlias(b.name, projectId, b.cohortId, 'se');
291+
return `${columnWithAlias} as b_${index}`;
292+
});
293+
const breakdownGroupBy = breakdowns.map((b, index) => `b_${index}`);
294+
295+
// Build LEFT JOINs for cohorts (on start_events)
296+
const cohortJoins = cohortIds.length > 0 ? '\n ' + cohortIds.map((cohortId) => {
206297
const cohortAlias = getCohortAlias(cohortId);
207298
const cohortCte = getCohortCteName(cohortId);
208-
return `LEFT ANY JOIN ${cohortCte} AS ${cohortAlias} ON ${cohortAlias}.profile_id = ${fromClause}.profile_id`;
209-
}).join('\n ') : '';
299+
return `LEFT ANY JOIN ${cohortCte} AS ${cohortAlias} ON ${cohortAlias}.profile_id = se.profile_id`;
300+
}).join('\n ') : '';
210301

211-
// Final step is the total number of events
212-
const finalStep = events.length;
302+
// Build WITH clause
303+
const withClause = ctes.length > 0 ? `WITH ${ctes.join(', ')} ` : '';
213304

214-
// Build windowFunnel query
215-
// Inner query uses table-qualified column (e.g., events.profile_id)
216-
// Outer query references the aliased column from subquery (group_id)
305+
// Build self-join query
217306
const query = clix(this.client, timezone)
218307
.select<{
219308
event_day: string;
@@ -224,48 +313,58 @@ export class ConversionService {
224313
}>([
225314
'event_day',
226315
...breakdownGroupBy,
227-
`uniqExact(group_id) AS total_first`,
228-
`countIf(steps >= ${finalStep}) AS conversions`,
229-
`round(100.0 * countIf(steps >= ${finalStep}) / uniqExact(group_id), 2) AS conversion_rate_percentage`,
316+
`uniqExact(${groupCol}) AS total_first`,
317+
`uniqExact(conversion_${groupCol}) AS conversions`,
318+
`round(100.0 * uniqExact(conversion_${groupCol}) / uniqExact(${groupCol}), 2) AS conversion_rate_percentage`,
230319
])
231320
.from(
232321
clix.exp(`
233322
(${withClause}SELECT
234-
${group} AS group_id,
235-
any(${clix.toStartOf('created_at', interval)}) as event_day,
236-
${breakdownColumns.length ? `${breakdownColumns.join(', ')},` : ''}
237-
windowFunnel(${funnelWindowSeconds})(
238-
toDateTime(created_at),
239-
${conditions.join(',\n ')}
240-
) as steps
241-
FROM ${fromClause}
242-
${cohortJoins}
243-
WHERE ${whereClauses.join(' AND ')}
244-
GROUP BY ${group}${breakdownGroupBy.length ? `, ${breakdownGroupBy.join(', ')}` : ''})
323+
${clix.toStartOf('se.created_at', interval)} as event_day,
324+
se.${groupCol},
325+
ee.${groupCol} as conversion_${groupCol}${breakdownColumns.length ? ',\n ' + breakdownColumns.join(',\n ') : ''}
326+
FROM start_events se
327+
LEFT JOIN end_events ee ON
328+
ee.${groupCol} = se.${groupCol}
329+
AND ee.created_at > se.created_at
330+
AND ee.created_at <= se.created_at + INTERVAL ${funnelWindowSeconds} SECOND
331+
${cohortJoins})
245332
`),
246333
)
247-
.where('steps', '>', 0)
248334
.groupBy(['event_day', ...breakdownGroupBy]);
249335

250336
for (const order of ['event_day', ...breakdownGroupBy]) {
251337
query.orderBy(order);
252338
}
253339

254340
const results = await query.execute();
255-
return this.toSeries(results, breakdowns, limit).map(
256-
(serie, serieIndex) => {
257-
return {
258-
...serie,
259-
data: serie.data.map((d, index) => ({
260-
...d,
261-
timestamp: new Date(d.date).getTime(),
262-
serieIndex,
263-
index,
264-
serie: omit(['data'], serie),
265-
})),
266-
};
267-
},
268-
);
341+
342+
// Sort series by average conversion rate (descending) when there are breakdowns
343+
const series = this.toSeries(results, breakdowns, limit);
344+
345+
if (breakdowns.length > 0) {
346+
series.sort((a, b) => {
347+
const avgRateA = a.data.reduce((sum, d) => sum + d.rate, 0) / (a.data.length || 1);
348+
const avgRateB = b.data.reduce((sum, d) => sum + d.rate, 0) / (b.data.length || 1);
349+
return avgRateB - avgRateA; // Descending order
350+
});
351+
}
352+
353+
// Apply limit after sorting
354+
const limitedSeries = limit && breakdowns.length > 0 ? series.slice(0, limit) : series;
355+
356+
return limitedSeries.map((serie, serieIndex) => {
357+
return {
358+
...serie,
359+
data: serie.data.map((d, index) => ({
360+
...d,
361+
timestamp: new Date(d.date).getTime(),
362+
serieIndex,
363+
index,
364+
serie: omit(['data'], serie),
365+
})),
366+
};
367+
});
269368
}
270369

271370
private toSeries(

0 commit comments

Comments
 (0)