Skip to content

Commit d90bccd

Browse files
Copilotmathiasrw
Copilot
and
mathiasrw
authored
Fix GROUP BY with aliased expressions to close #1968 (#2398)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mathiasrw <1063454+mathiasrw@users.noreply.github.com> Co-authored-by: M. Wulff <m@rawu.dk>
1 parent c6b6e20 commit d90bccd

File tree

3 files changed

+207
-0
lines changed

3 files changed

+207
-0
lines changed

‎.prettierignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,7 @@ src/97saveas.js
1313
modules/
1414

1515
test/test238.json
16+
17+
build/
18+
19+
test/lib/

‎src/424select.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,31 @@ yy.Select.prototype.compileSelect2 = function (query, params) {
473473

474474
yy.Select.prototype.compileSelectGroup0 = function (query) {
475475
var self = this;
476+
477+
// Optimization: Build lookup structures upfront to avoid O(n*m) complexity in the main loop
478+
// Only build these if GROUP BY exists, as they're only used for alias resolution
479+
var groupByAliasMap = null;
480+
var selectColumnNames = null;
481+
482+
if (self.group) {
483+
// Build map of GROUP BY columns that reference aliases (for O(1) lookup)
484+
groupByAliasMap = {};
485+
self.group.forEach(function (gp, idx) {
486+
if (gp instanceof yy.Column && gp.columnid && !gp.tableid) {
487+
groupByAliasMap[gp.columnid] = idx;
488+
}
489+
});
490+
491+
// Build set of actual column names in SELECT to distinguish pure aliases from column renames
492+
// This prevents incorrect replacement of "GROUP BY b" when "SELECT a AS b, b AS c" exists
493+
selectColumnNames = {};
494+
self.columns.forEach(function (col) {
495+
if (col instanceof yy.Column && col.columnid) {
496+
selectColumnNames[col.columnid] = true;
497+
}
498+
});
499+
}
500+
476501
self.columns.forEach(function (col, idx) {
477502
if (!(col instanceof yy.Column && col.columnid === '*')) {
478503
var colas;
@@ -493,12 +518,35 @@ yy.Select.prototype.compileSelectGroup0 = function (query) {
493518
col.nick = colas;
494519

495520
if (self.group) {
521+
// Match GROUP BY columns to SELECT columns by columnid and tableid (for real columns)
496522
var groupIdx = self.group.findIndex(function (gp) {
497523
return gp.columnid === col.columnid && gp.tableid === col.tableid;
498524
});
499525
if (groupIdx > -1) {
500526
self.group[groupIdx].nick = colas;
501527
}
528+
529+
// Also match GROUP BY columns that reference SELECT column aliases
530+
// This handles cases like: SELECT CASE ... END AS age_group ... GROUP BY age_group
531+
// Only apply if:
532+
// 1. The SELECT column has an alias
533+
// 2. That alias matches a GROUP BY column name
534+
// 3. The alias is NOT an actual column name (pure alias, not renaming)
535+
if (
536+
col.as &&
537+
groupByAliasMap &&
538+
groupByAliasMap.hasOwnProperty(col.as) &&
539+
!selectColumnNames[col.as]
540+
) {
541+
var aliasGroupIdx = groupByAliasMap[col.as];
542+
// Replace the GROUP BY column reference with a deep copy of the SELECT expression
543+
// We use deep cloning to ensure nested objects (like CASE whens/elses) are copied
544+
var groupExpr = cloneDeep(col);
545+
// Clear SELECT-specific properties that shouldn't be in GROUP BY
546+
delete groupExpr.as;
547+
groupExpr.nick = colas;
548+
self.group[aliasGroupIdx] = groupExpr;
549+
}
502550
}
503551

504552
if (

‎test/test1146.js‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
if (typeof exports === 'object') {
2+
var assert = require('assert');
3+
var alasql = require('..');
4+
}
5+
6+
describe('Test 2361 - GROUP BY with CASE expression alias', function () {
7+
const test = '2361';
8+
9+
before(function () {
10+
alasql('create database test' + test);
11+
alasql('use test' + test);
12+
});
13+
14+
after(function () {
15+
alasql('drop database test' + test);
16+
});
17+
18+
it('A) GROUP BY with CASE WHEN aliased expression', function () {
19+
// Create test data with ages
20+
var data = [{age: 25}, {age: 26}, {age: 35}, {age: 36}, {age: 45}, {age: 55}];
21+
22+
var result = alasql(
23+
`SELECT
24+
CASE
25+
WHEN age BETWEEN 20 AND 29 THEN '20-29'
26+
WHEN age BETWEEN 30 AND 39 THEN '30-39'
27+
WHEN age BETWEEN 40 AND 49 THEN '40-49'
28+
WHEN age BETWEEN 50 AND 59 THEN '50-59'
29+
ELSE '60+'
30+
END AS age_group,
31+
COUNT(*) AS customer_count
32+
FROM ?
33+
GROUP BY age_group
34+
ORDER BY age_group`,
35+
[data]
36+
);
37+
38+
var expected = [
39+
{age_group: '20-29', customer_count: 2},
40+
{age_group: '30-39', customer_count: 2},
41+
{age_group: '40-49', customer_count: 1},
42+
{age_group: '50-59', customer_count: 1},
43+
];
44+
45+
assert.deepEqual(result, expected);
46+
});
47+
48+
it('B) GROUP BY with CASE WHEN and ELSE clause', function () {
49+
var data = [{age: 10}, {age: 20}, {age: 30}, {age: 100}];
50+
51+
var result = alasql(
52+
`SELECT
53+
CASE
54+
WHEN age BETWEEN 0 AND 9 THEN '0-9'
55+
WHEN age BETWEEN 10 AND 19 THEN '10-19'
56+
WHEN age BETWEEN 20 AND 29 THEN '20-29'
57+
ELSE '30+'
58+
END AS age_group
59+
FROM ?
60+
GROUP BY age_group`,
61+
[data]
62+
);
63+
64+
// Should return three unique groups, not just '30+'
65+
var expected = [{age_group: '10-19'}, {age_group: '20-29'}, {age_group: '30+'}];
66+
67+
assert.deepEqual(result.sort(), expected.sort());
68+
});
69+
70+
it('C) GROUP BY with function expression alias', function () {
71+
var data = [{name: 'Alice'}, {name: 'alice'}, {name: 'Bob'}, {name: 'bob'}];
72+
73+
var result = alasql(
74+
`SELECT
75+
UPPER(name) AS upper_name,
76+
COUNT(*) AS cnt
77+
FROM ?
78+
GROUP BY upper_name
79+
ORDER BY upper_name`,
80+
[data]
81+
);
82+
83+
var expected = [
84+
{upper_name: 'ALICE', cnt: 2},
85+
{upper_name: 'BOB', cnt: 2},
86+
];
87+
88+
assert.deepEqual(result, expected);
89+
});
90+
91+
it('D) GROUP BY with multiple CASE expressions', function () {
92+
var data = [
93+
{age: 25, score: 85},
94+
{age: 26, score: 90},
95+
{age: 35, score: 85},
96+
{age: 36, score: 90},
97+
];
98+
99+
var result = alasql(
100+
`SELECT
101+
CASE
102+
WHEN age BETWEEN 20 AND 29 THEN '20-29'
103+
ELSE '30+'
104+
END AS age_group,
105+
CASE
106+
WHEN score >= 90 THEN 'High'
107+
ELSE 'Low'
108+
END AS score_group,
109+
COUNT(*) AS cnt
110+
FROM ?
111+
GROUP BY age_group, score_group
112+
ORDER BY age_group, score_group`,
113+
[data]
114+
);
115+
116+
var expected = [
117+
{age_group: '20-29', score_group: 'High', cnt: 1},
118+
{age_group: '20-29', score_group: 'Low', cnt: 1},
119+
{age_group: '30+', score_group: 'High', cnt: 1},
120+
{age_group: '30+', score_group: 'Low', cnt: 1},
121+
];
122+
123+
assert.deepEqual(result, expected);
124+
});
125+
126+
it('E) GROUP BY with WHERE and CASE expression alias', function () {
127+
var data = [
128+
{age: 25, active: true},
129+
{age: 26, active: false},
130+
{age: 35, active: true},
131+
{age: 36, active: true},
132+
];
133+
134+
var result = alasql(
135+
`SELECT
136+
CASE
137+
WHEN age BETWEEN 20 AND 29 THEN '20-29'
138+
ELSE '30+'
139+
END AS age_group,
140+
COUNT(*) AS cnt
141+
FROM ?
142+
WHERE active = true
143+
GROUP BY age_group
144+
ORDER BY age_group`,
145+
[data]
146+
);
147+
148+
var expected = [
149+
{age_group: '20-29', cnt: 1},
150+
{age_group: '30+', cnt: 2},
151+
];
152+
153+
assert.deepEqual(result, expected);
154+
});
155+
});

0 commit comments

Comments
 (0)