fix group issues

This commit is contained in:
Carl-Gerhard Lindesvärd
2026-03-16 23:33:05 +01:00
parent 995f32c5d8
commit 2dc622cbf2
5 changed files with 63 additions and 50 deletions

View File

@@ -20,11 +20,17 @@ export const average = (arr: (number | null)[], includeZero = false) => {
export const sum = (arr: (number | null | undefined)[]): number =>
round(arr.filter(isNumber).reduce((acc, item) => acc + item, 0));
export const min = (arr: (number | null | undefined)[]): number =>
Math.min(...arr.filter(isNumber));
export const min = (arr: (number | null | undefined)[]): number => {
const filtered = arr.filter(isNumber);
if (filtered.length === 0) return 0;
return filtered.reduce((a, b) => (b < a ? b : a), filtered[0]!);
};
export const max = (arr: (number | null | undefined)[]): number =>
Math.max(...arr.filter(isNumber));
export const max = (arr: (number | null | undefined)[]): number => {
const filtered = arr.filter(isNumber);
if (filtered.length === 0) return 0;
return filtered.reduce((a, b) => (b > a ? b : a), filtered[0]!);
};
export const isFloat = (n: number) => n % 1 !== 0;

View File

@@ -26,7 +26,7 @@ export function format(
}>,
includeAlphaIds: boolean,
previousSeries: ConcreteSeries[] | null = null,
limit: number | undefined = undefined,
limit: number | undefined = undefined
): FinalChart {
const series = concreteSeries.map((cs) => {
// Find definition for this series
@@ -70,7 +70,7 @@ export function format(
const previousSerie = previousSeries?.find(
(ps) =>
ps.definitionIndex === cs.definitionIndex &&
ps.name.slice(1).join(':::') === cs.name.slice(1).join(':::'),
ps.name.slice(1).join(':::') === cs.name.slice(1).join(':::')
);
return {
@@ -89,24 +89,24 @@ export function format(
previous: {
sum: getPreviousMetric(
metrics.sum,
sum(previousSerie.data.map((d) => d.count)),
sum(previousSerie.data.map((d) => d.count))
),
average: getPreviousMetric(
metrics.average,
round(average(previousSerie.data.map((d) => d.count)), 2),
round(average(previousSerie.data.map((d) => d.count)), 2)
),
min: getPreviousMetric(
metrics.min,
min(previousSerie.data.map((d) => d.count)),
min(previousSerie.data.map((d) => d.count))
),
max: getPreviousMetric(
metrics.max,
max(previousSerie.data.map((d) => d.count)),
max(previousSerie.data.map((d) => d.count))
),
count: getPreviousMetric(
metrics.count ?? 0,
previousSerie.data.find((item) => !!item.total_count)
?.total_count ?? null,
?.total_count ?? null
),
},
}
@@ -118,7 +118,7 @@ export function format(
previous: previousSerie?.data[index]
? getPreviousMetric(
item.count,
previousSerie.data[index]?.count ?? null,
previousSerie.data[index]?.count ?? null
)
: undefined,
})),

View File

@@ -150,7 +150,7 @@ export function getChartSql({
if (event.name !== '*') {
sb.select.label_0 = `${sqlstring.escape(event.name)} as label_0`;
sb.where.eventName = `name = ${sqlstring.escape(event.name)}`;
sb.where.eventName = `e.name = ${sqlstring.escape(event.name)}`;
} else {
sb.select.label_0 = `'*' as label_0`;
}
@@ -359,7 +359,7 @@ export function getChartSql({
if (event.segment === 'one_event_per_user') {
sb.from = `(
SELECT DISTINCT ON (profile_id) * from ${TABLE_NAMES.events} ${getJoins()} WHERE ${join(
SELECT DISTINCT ON (profile_id) * from ${TABLE_NAMES.events} e ${getJoins()} WHERE ${join(
sb.where,
' AND '
)}
@@ -380,40 +380,47 @@ export function getChartSql({
: '';
if (breakdowns.length > 0) {
// Match breakdown properties in subquery with outer query's grouped values
// Since outer query groups by label_X, we reference those in the correlation
const breakdownMatches = breakdowns
// Pre-compute unique counts per breakdown group in a CTE, then JOIN it.
// We can't use a correlated subquery because:
// 1. ClickHouse expands label_X aliases to their underlying expressions,
// which resolve in the subquery's scope, making the condition a tautology.
// 2. Correlated subqueries aren't supported on distributed/remote tables.
const ucSelectParts: string[] = breakdowns.map((breakdown, index) => {
const propertyKey = getSelectPropertyKey(breakdown.name, projectId);
return `${propertyKey} as _uc_label_${index + 1}`;
});
ucSelectParts.push('uniq(profile_id) as total_count');
const ucGroupByParts = breakdowns.map(
(_, index) => `_uc_label_${index + 1}`
);
const ucWhere = getWhereWithoutBar();
addCte(
'_uc',
`SELECT ${ucSelectParts.join(', ')} FROM ${TABLE_NAMES.events} e ${subqueryGroupJoins}${profilesJoinRef ? `${profilesJoinRef} ` : ''}${ucWhere} GROUP BY ${ucGroupByParts.join(', ')}`
);
const ucJoinConditions = breakdowns
.map((b, index) => {
const propertyKey = getSelectPropertyKey(b.name, projectId);
// Correlate: match the property expression with outer query's label_X value
// ClickHouse allows referencing outer query columns in correlated subqueries
return `${propertyKey} = label_${index + 1}`;
return `_uc._uc_label_${index + 1} = ${propertyKey}`;
})
.join(' AND ');
// Build WHERE clause for subquery - replace table alias and keep profile CTE reference
const subqueryWhere = getWhereWithoutBar()
.replace(/\be\./g, 'e2.')
.replace(/\bprofile\./g, 'profile.');
sb.select.total_unique_count = `(
SELECT uniq(profile_id)
FROM ${TABLE_NAMES.events} e2
${subqueryGroupJoins}${profilesJoinRef ? `${profilesJoinRef} ` : ''}${subqueryWhere}
AND ${breakdownMatches}
) as total_count`;
sb.joins.unique_counts = `LEFT ANY JOIN _uc ON ${ucJoinConditions}`;
sb.select.total_unique_count = 'any(_uc.total_count) as total_count';
} else {
// No breakdowns: calculate unique count across all data
// Build WHERE clause for subquery - replace table alias and keep profile CTE reference
const subqueryWhere = getWhereWithoutBar()
.replace(/\be\./g, 'e2.')
.replace(/\bprofile\./g, 'profile.');
const ucWhere = getWhereWithoutBar();
sb.select.total_unique_count = `(
SELECT uniq(profile_id)
FROM ${TABLE_NAMES.events} e2
${subqueryGroupJoins}${profilesJoinRef ? `${profilesJoinRef} ` : ''}${subqueryWhere}
) as total_count`;
addCte(
'_uc',
`SELECT uniq(profile_id) as total_count FROM ${TABLE_NAMES.events} e ${subqueryGroupJoins}${profilesJoinRef ? `${profilesJoinRef} ` : ''}${ucWhere}`
);
sb.select.total_unique_count =
'(SELECT total_count FROM _uc) as total_count';
}
const sql = `${getWith()}${getSelect()} ${getFrom()} ${getJoins()} ${getWhere()} ${getGroupBy()} ${getOrderBy()} ${getFill()}`;
@@ -440,7 +447,7 @@ export function getAggregateChartSql({
if (event.name !== '*') {
sb.select.label_0 = `${sqlstring.escape(event.name)} as label_0`;
sb.where.eventName = `name = ${sqlstring.escape(event.name)}`;
sb.where.eventName = `e.name = ${sqlstring.escape(event.name)}`;
} else {
sb.select.label_0 = `'*' as label_0`;
}
@@ -617,7 +624,7 @@ export function getAggregateChartSql({
if (event.segment === 'one_event_per_user') {
sb.from = `(
SELECT DISTINCT ON (profile_id) * from ${TABLE_NAMES.events} ${getJoins()} WHERE ${join(
SELECT DISTINCT ON (profile_id) * from ${TABLE_NAMES.events} e ${getJoins()} WHERE ${join(
sb.where,
' AND '
)}

View File

@@ -101,11 +101,11 @@ export class ConversionService {
// Build funnel conditions
const conditionA = whereA
? `(name = '${eventA.name}' AND ${whereA})`
: `name = '${eventA.name}'`;
? `(events.name = '${eventA.name}' AND ${whereA})`
: `events.name = '${eventA.name}'`;
const conditionB = whereB
? `(name = '${eventB.name}' AND ${whereB})`
: `name = '${eventB.name}'`;
? `(events.name = '${eventB.name}' AND ${whereB})`
: `events.name = '${eventB.name}'`;
const groupJoin = needsGroupArrayJoin
? `ARRAY JOIN groups AS _group_id LEFT ANY JOIN (SELECT id, name, type, properties FROM ${TABLE_NAMES.groups} FINAL WHERE project_id = ${sqlstring.escape(projectId)}) AS _g ON _g.id = _group_id`
@@ -141,7 +141,7 @@ export class ConversionService {
${profileJoin}
${groupJoin}
WHERE project_id = '${projectId}'
AND name IN ('${eventA.name}', '${eventB.name}')
AND events.name IN ('${eventA.name}', '${eventB.name}')
AND created_at BETWEEN toDateTime('${startDate}') AND toDateTime('${endDate}')
GROUP BY ${group}${breakdownExpressions.length ? `, ${breakdownExpressions.join(', ')}` : ''})
`),

View File

@@ -38,7 +38,7 @@ export class FunnelService {
return events.map((event) => {
const { sb, getWhere } = createSqlBuilder();
sb.where = getEventFiltersWhereClause(event.filters, projectId);
sb.where.name = `name = ${sqlstring.escape(event.name)}`;
sb.where.name = `events.name = ${sqlstring.escape(event.name)}`;
return getWhere().replace('WHERE ', '');
});
}
@@ -90,7 +90,7 @@ export class FunnelService {
clix.datetime(endDate, 'toDateTime'),
])
.where(
'name',
'events.name',
'IN',
eventSeries.map((e) => e.name),
)