Skip to content

Commit a6e030f

Browse files
Copilotmathiasrw
andauthored
Fix output when reusing aggregate function to close 1147 (#2405)
When the same aggregate function appears multiple times in a `SELECT statement (e.g., sum(population) AS val3, sum(population) AS val4)`, they were incorrectly accumulating into separate accumulators, leading to different results. --------- 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: Mathias Wulff <m@rawu.dk>
1 parent c94f387 commit a6e030f

2 files changed

Lines changed: 127 additions & 10 deletions

File tree

src/50expression.js

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -834,17 +834,23 @@
834834
}
835835

836836
findAggregator(query) {
837-
const colas = escapeq(this.toString()) + ':' + query.selectGroup.length;
838-
839-
if (!this.nick) {
840-
this.nick = colas;
841-
842-
if (!query.removeKeys.includes(colas)) {
843-
query.removeKeys.push(colas);
837+
// Check if an identical aggregate already exists in selectGroup
838+
let existingAggr = query.selectGroup.find(agg => agg.toString() === this.toString());
839+
840+
if (existingAggr) {
841+
// Reuse the existing aggregate's nick to share the same accumulator
842+
this.aggrNick = existingAggr.nick;
843+
} else {
844+
// New aggregate - assign nick and add to selectGroup
845+
if (!this.nick) {
846+
this.nick = escapeq(this.toString()) + ':' + query.selectGroup.length;
847+
if (!query.removeKeys.includes(this.nick)) {
848+
query.removeKeys.push(this.nick);
849+
}
844850
}
851+
this.aggrNick = this.nick;
852+
query.selectGroup.push(this);
845853
}
846-
847-
query.selectGroup.push(this);
848854
}
849855

850856
toType() {
@@ -864,7 +870,8 @@
864870
}
865871

866872
toJS() {
867-
var colas = this.nick;
873+
// Use aggrNick for duplicate aggregates to share the same accumulator, otherwise use the unique nick
874+
var colas = this.aggrNick || this.nick;
868875
if (colas === undefined) {
869876
colas = escapeq(this.toString());
870877
}

test/test1147.js

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
if (typeof exports === 'object') {
2+
var assert = require('assert');
3+
var alasql = require('..');
4+
}
5+
6+
describe('Test 942 - Duplicate aggregate functions return different values', function () {
7+
const test = '942';
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) Two identical SUM aggregates without rownum should return same value', function () {
19+
alasql('CREATE TABLE test (population number)');
20+
alasql('INSERT INTO test VALUES (100), (200), (300)');
21+
22+
var res = alasql('SELECT sum(population) as val1, sum(population) as val2 FROM test');
23+
24+
assert.deepEqual(res, [{val1: 600, val2: 600}]);
25+
26+
alasql('DROP TABLE test');
27+
});
28+
29+
it('B) Two identical SUM aggregates with rownum should return same value', function () {
30+
alasql('CREATE TABLE test (city string, price number, people number, population number)');
31+
alasql(
32+
"INSERT INTO test VALUES ('Rome',10,1,2863223),('Paris',20,1,2249975),('Berlin',30,1,3517424), ('Madrid',40,1,3041579)"
33+
);
34+
35+
var res = alasql(
36+
'SELECT sum(population) as val3, sum(population) as val4, rownum() as rownum FROM test'
37+
);
38+
39+
assert.deepEqual(res, [{val3: 11672201, val4: 11672201, rownum: 1}]);
40+
41+
alasql('DROP TABLE test');
42+
});
43+
44+
it('C) Multiple identical aggregates in complex query', function () {
45+
alasql('CREATE TABLE test (city string, price number, people number, population number)');
46+
alasql(
47+
"INSERT INTO test VALUES ('Rome',10,1,2863223),('Paris',20,1,2249975),('Berlin',30,1,3517424), ('Madrid',40,1,3041579)"
48+
);
49+
50+
var sql =
51+
'SELECT ' +
52+
'CASE WHEN sum(people)=0 THEN 0 ELSE (sum(cast(price as float)) / sum(cast(people as float))) END as val1, ' +
53+
'CASE WHEN sum(population)=0 THEN 0 ELSE (sum(cast(price as float)) / sum(cast(population as float))) END as val2, ' +
54+
'sum(population) as val3, ' +
55+
'sum(population) as val4, ' +
56+
'rownum() as rownum ' +
57+
'FROM test';
58+
59+
var res = alasql(sql);
60+
61+
assert.deepEqual(res, [
62+
{
63+
val1: 25,
64+
val2: 0.000008567364458511296,
65+
val3: 11672201,
66+
val4: 11672201,
67+
rownum: 1,
68+
},
69+
]);
70+
71+
alasql('DROP TABLE test');
72+
});
73+
74+
it('D) Mixed aggregates with some duplicates', function () {
75+
alasql('CREATE TABLE test (a int, b int, c int)');
76+
alasql('INSERT INTO test VALUES (1,10,100),(2,20,200),(3,30,300)');
77+
78+
var res = alasql(
79+
'SELECT sum(a) as sum1, sum(a) as sum2, sum(b) as sum3, count(a) as cnt1, count(b) as cnt2, avg(a) as avg1 FROM test'
80+
);
81+
82+
assert.deepEqual(res, [{sum1: 6, sum2: 6, sum3: 60, cnt1: 3, cnt2: 3, avg1: 2}]);
83+
84+
alasql('DROP TABLE test');
85+
});
86+
87+
it('E) Multiple duplicate aggregates with different functions', function () {
88+
alasql('CREATE TABLE test (x int, y int)');
89+
alasql('INSERT INTO test VALUES (5,10),(15,20),(25,30)');
90+
91+
var res = alasql(
92+
'SELECT sum(x) as s1, sum(x) as s2, sum(y) as s3, sum(y) as s4, min(x) as min1, min(x) as min2, max(y) as max1, max(y) as max2 FROM test'
93+
);
94+
95+
assert.deepEqual(res, [{s1: 45, s2: 45, s3: 60, s4: 60, min1: 5, min2: 5, max1: 30, max2: 30}]);
96+
97+
alasql('DROP TABLE test');
98+
});
99+
100+
it('F) Duplicate aggregates in SELECT ROW modifier', function () {
101+
alasql('CREATE TABLE test (val int)');
102+
alasql('INSERT INTO test VALUES (1),(2),(3),(4),(5)');
103+
104+
var res = alasql('SELECT ROW sum(val), sum(val), count(val), count(val) FROM test');
105+
106+
assert.deepEqual(res, [15, 15, 5, 5]);
107+
108+
alasql('DROP TABLE test');
109+
});
110+
});

0 commit comments

Comments
 (0)