Skip to content

Commit fc10673

Browse files
Copilotmathiasrw
Copilot
and
mathiasrw
authored
Correct grouping across columns with same name to fix #941 (#2307)
Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: mathiasrw <[email protected]>
1 parent d206837 commit fc10673

File tree

3 files changed

+212
-4
lines changed

3 files changed

+212
-4
lines changed

‎src/38query.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,25 @@ function queryfn3(query) {
140140
var d = query.selectgfn(g, query.params, alasql);
141141

142142
for (const key in query.groupColumns) {
143-
// ony remove columns where the alias is also not a column in the result
143+
// only remove columns where the alias is also not a column in the result
144+
// and no other alias in the result also points to the same nick
144145
if (
145146
query.groupColumns[key] !== key &&
146147
d[query.groupColumns[key]] &&
147148
!query.groupColumns[query.groupColumns[key]]
148149
) {
149-
delete d[query.groupColumns[key]];
150+
// Check if any other key in the result also maps to this nick
151+
var nick = query.groupColumns[key];
152+
var otherAliasExists = false;
153+
for (const otherKey in query.groupColumns) {
154+
if (otherKey !== key && query.groupColumns[otherKey] === nick && d[otherKey]) {
155+
otherAliasExists = true;
156+
break;
157+
}
158+
}
159+
if (!otherAliasExists) {
160+
delete d[query.groupColumns[key]];
161+
}
150162
}
151163
}
152164
query.data.push(d);

‎src/424select.js‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,10 +608,34 @@ yy.Select.prototype.compileSelectGroup1 = function (query) {
608608
yy.Select.prototype.compileSelectGroup2 = function (query) {
609609
var self = this;
610610
var s = query.selectgfns;
611+
612+
// Create a lookup map for GROUP BY columns to optimize performance
613+
var groupColMap = {};
614+
if (self.group) {
615+
self.group.forEach(function (gp) {
616+
var key = (gp.tableid || '') + '\t' + gp.columnid;
617+
groupColMap[key] = gp;
618+
});
619+
}
620+
611621
self.columns.forEach(function (col) {
612622
// console.log(col);
613-
if (query.ingroup.indexOf(col.nick) > -1) {
614-
s += "r['" + (col.as || col.nick) + "']=g['" + col.nick + "'];";
623+
// Skip SELECT * columns as they are handled differently
624+
if (col instanceof yy.Column && col.columnid === '*') {
625+
return;
626+
}
627+
// Check if this column is part of GROUP BY
628+
// For columns with renamed nicks (e.g., 'x:1'), we need to check the original columnid
629+
var groupCol = null;
630+
if (col instanceof yy.Column && self.group) {
631+
var key = (col.tableid || '') + '\t' + col.columnid;
632+
groupCol = groupColMap[key];
633+
}
634+
var isInGroup = groupCol !== null || query.ingroup.indexOf(col.nick) > -1;
635+
if (isInGroup) {
636+
// For columns in GROUP BY, use the GROUP BY column's nick if available
637+
var groupNick = (groupCol && groupCol.nick) || col.nick;
638+
s += "r['" + (col.as || col.nick) + "']=g['" + groupNick + "'];";
615639
}
616640
});
617641

‎test/test941.js‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
if (typeof exports === 'object') {
2+
var assert = require('assert');
3+
var alasql = require('..');
4+
}
5+
6+
describe('Test 941 - GROUP BY with duplicate column names', function () {
7+
it('A) GROUP BY on same column twice with different aliases', function () {
8+
var accounts = [
9+
{name: 'A', region_id: 1},
10+
{name: 'B', region_id: 2},
11+
];
12+
13+
var result = alasql(
14+
'SELECT accounts.name AS `AccountName`, accounts.name AS `AccountName2`, COUNT(1) AS `Count` FROM ? accounts GROUP BY accounts.name',
15+
[accounts]
16+
);
17+
18+
var expected = [
19+
{AccountName: 'A', AccountName2: 'A', Count: 1},
20+
{AccountName: 'B', AccountName2: 'B', Count: 1},
21+
];
22+
assert.deepEqual(result, expected);
23+
});
24+
25+
it('B) GROUP BY with join on columns with same name', function () {
26+
var accounts = [
27+
{name: 'A', region_id: 1},
28+
{name: 'B', region_id: 2},
29+
];
30+
31+
var regions = [
32+
{id: 1, name: 'North'},
33+
{id: 2, name: 'South'},
34+
];
35+
36+
var result = alasql(
37+
'SELECT accounts.name AS `AccountName`, regions.name AS `RegionName`, COUNT(1) AS `Count` FROM ? accounts LEFT JOIN ? regions ON accounts.region_id = regions.id GROUP BY accounts.name, regions.name',
38+
[accounts, regions]
39+
);
40+
41+
var expected = [
42+
{AccountName: 'A', RegionName: 'North', Count: 1},
43+
{AccountName: 'B', RegionName: 'South', Count: 1},
44+
];
45+
assert.deepEqual(result, expected);
46+
});
47+
48+
it('C) GROUP BY on same column three times', function () {
49+
var data = [{x: 1}, {x: 2}];
50+
51+
var result = alasql('SELECT x AS a, x AS b, x AS c, COUNT(1) AS cnt FROM ? GROUP BY x', [data]);
52+
53+
var expected = [
54+
{a: 1, b: 1, c: 1, cnt: 1},
55+
{a: 2, b: 2, c: 2, cnt: 1},
56+
];
57+
assert.deepEqual(result, expected);
58+
});
59+
60+
it('D) SELECT * with GROUP BY and duplicate column in SELECT', function () {
61+
var data = [
62+
{x: 1, y: 'a'},
63+
{x: 1, y: 'b'},
64+
{x: 2, y: 'c'},
65+
];
66+
67+
var result = alasql('SELECT *, x AS x_copy, COUNT(1) AS cnt FROM ? GROUP BY x', [data]);
68+
69+
var expected = [
70+
{x: 1, y: 'a', x_copy: 1, cnt: 2},
71+
{x: 2, y: 'c', x_copy: 2, cnt: 1},
72+
];
73+
assert.deepEqual(result, expected);
74+
});
75+
76+
it('E) Duplicate column with different aggregates', function () {
77+
var data = [
78+
{x: 1, y: 10},
79+
{x: 1, y: 20},
80+
{x: 2, y: 30},
81+
];
82+
83+
var result = alasql(
84+
'SELECT x AS grp1, x AS grp2, MAX(y) AS max_y, MIN(y) AS min_y FROM ? GROUP BY x',
85+
[data]
86+
);
87+
88+
var expected = [
89+
{grp1: 1, grp2: 1, max_y: 20, min_y: 10},
90+
{grp1: 2, grp2: 2, max_y: 30, min_y: 30},
91+
];
92+
assert.deepEqual(result, expected);
93+
});
94+
95+
it('F) GROUP BY multiple columns with duplicates in SELECT', function () {
96+
var data = [
97+
{a: 1, b: 'x', c: 100},
98+
{a: 1, b: 'y', c: 200},
99+
{a: 2, b: 'x', c: 300},
100+
];
101+
102+
var result = alasql(
103+
'SELECT a AS a1, a AS a2, b AS b1, b AS b2, SUM(c) AS sum_c FROM ? GROUP BY a, b',
104+
[data]
105+
);
106+
107+
var expected = [
108+
{a1: 1, a2: 1, b1: 'x', b2: 'x', sum_c: 100},
109+
{a1: 1, a2: 1, b1: 'y', b2: 'y', sum_c: 200},
110+
{a1: 2, a2: 2, b1: 'x', b2: 'x', sum_c: 300},
111+
];
112+
assert.deepEqual(result, expected);
113+
});
114+
115+
it('G) Duplicate columns with HAVING clause', function () {
116+
var data = [{x: 1}, {x: 1}, {x: 2}];
117+
118+
var result = alasql(
119+
'SELECT x AS col1, x AS col2, COUNT(1) AS cnt FROM ? GROUP BY x HAVING COUNT(1) > 1',
120+
[data]
121+
);
122+
123+
var expected = [{col1: 1, col2: 1, cnt: 2}];
124+
assert.deepEqual(result, expected);
125+
});
126+
127+
it('H) Duplicate columns in subquery with GROUP BY', function () {
128+
var data = [{x: 1}, {x: 1}, {x: 2}];
129+
130+
var result = alasql(
131+
'SELECT col1, col2, cnt FROM (SELECT x AS col1, x AS col2, COUNT(1) AS cnt FROM ? GROUP BY x) WHERE cnt > 0',
132+
[data]
133+
);
134+
135+
var expected = [
136+
{col1: 1, col2: 1, cnt: 2},
137+
{col1: 2, col2: 2, cnt: 1},
138+
];
139+
assert.deepEqual(result, expected);
140+
});
141+
142+
it('I) Multiple duplicate columns from different aggregations', function () {
143+
var data = [
144+
{x: 1, a: 10, b: 5},
145+
{x: 1, a: 20, b: 15},
146+
{x: 2, a: 30, b: 25},
147+
];
148+
149+
var result = alasql(
150+
'SELECT x AS id1, x AS id2, SUM(a) AS sum_a, SUM(b) AS sum_b, AVG(a) AS avg_a FROM ? GROUP BY x',
151+
[data]
152+
);
153+
154+
var expected = [
155+
{id1: 1, id2: 1, sum_a: 30, sum_b: 20, avg_a: 15},
156+
{id1: 2, id2: 2, sum_a: 30, sum_b: 25, avg_a: 30},
157+
];
158+
assert.deepEqual(result, expected);
159+
});
160+
161+
it('J) Duplicate columns with DISTINCT', function () {
162+
var data = [{x: 1}, {x: 1}, {x: 2}];
163+
164+
var result = alasql('SELECT DISTINCT x AS col1, x AS col2 FROM ? ORDER BY x', [data]);
165+
166+
var expected = [
167+
{col1: 1, col2: 1},
168+
{col1: 2, col2: 2},
169+
];
170+
assert.deepEqual(result, expected);
171+
});
172+
});

0 commit comments

Comments
 (0)