From b1ca96f1de14489a85e106ea68b03598e2117fd2 Mon Sep 17 00:00:00 2001 From: Joshua <143388422+joshuablac@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:05:31 +0100 Subject: [PATCH] Fix CUBE and GROUPING SETS merging groups from different sets to close #644 Groups were keyed only by the values of their grouping columns, so with several grouping sets a group from one set could collide with a group from another set that had the same values, e.g. (a=2) from CUBE(a, b) and (b=2). The rows were then summed into one group and the other group was missing. The group key now includes the grouping set index when the query has more than one grouping set. Plain GROUP BY keys are unchanged. test015 asserted the merged totals; it now expects the correct eight groups. Co-Authored-By: Claude Opus 5.5 --- src/423groupby.js | 13 ++++++--- test/test015.js | 16 +++++++---- test/test644.js | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 test/test644.js diff --git a/src/423groupby.js b/src/423groupby.js index 39461210fe..3dd2c9034c 100755 --- a/src/423groupby.js +++ b/src/423groupby.js @@ -51,7 +51,7 @@ yy.Select.prototype.compileGroup = function (query) { query.ingroup = []; var s = ''; - allgroup.forEach(function (agroup) { + allgroup.forEach(function (agroup, setIdx) { // Start of group function s += 'var g=this.xgroups['; @@ -71,9 +71,16 @@ yy.Select.prototype.compileGroup = function (query) { rg = ["''"]; } - s += rg.join('+"`"+'); + // With several grouping sets (CUBE, ROLLUP, GROUPING SETS) the same values can + // appear in different sets, e.g. (a=2) and (b=2), so the key must include the set + var groupKey = rg.join('+"`"+'); + if (allgroup.length > 1) { + groupKey = `'${setIdx}\`'+` + groupKey; + } + + s += groupKey; s += '];if(!g) {this.groups.push((g=this.xgroups['; - s += rg.join('+"`"+'); + s += groupKey; s += '] = {'; s += agroup .map(function (col2) { diff --git a/test/test015.js b/test/test015.js index ba5ab60b2b..2546068a1b 100644 --- a/test/test015.js +++ b/test/test015.js @@ -21,15 +21,21 @@ describe('Test 15', function () { 'SELECT a,b,c, SUM(d) AS d FROM test ' + ' GROUP BY GROUPING SETS(a,b),c ORDER BY a,b,c' ); + // Grouping sets (a,c) and (b,c) are separate groups (see #644) var tobe = [ - {a: 0, b: null, c: 0, d: 1000}, - {a: 0, b: null, c: 1, d: 1400}, - {a: null, b: 1, c: 0, d: 2200}, - {a: null, b: 1, c: 1, d: 2600}, + {a: 0, b: null, c: 0, d: 400}, + {a: null, b: 0, c: 0, d: 600}, + {a: 0, b: null, c: 1, d: 600}, + {a: null, b: 0, c: 1, d: 800}, + {a: null, b: 1, c: 0, d: 1000}, + {a: null, b: 1, c: 1, d: 1200}, + {a: 1, b: null, c: 0, d: 1200}, + {a: 1, b: null, c: 1, d: 1400}, ]; + assert.equal(res.length, tobe.length); var check = true; - for (var i = 0; i < 4; i++) { + for (var i = 0; i < tobe.length; i++) { for (var p in tobe[i]) { check = check && res[i][p] == tobe[i][p]; } diff --git a/test/test644.js b/test/test644.js new file mode 100644 index 0000000000..1815ace73a --- /dev/null +++ b/test/test644.js @@ -0,0 +1,69 @@ +if (typeof exports === 'object') { + var assert = require('assert'); + var alasql = require('..'); +} + +let testId = '644'; // GROUP BY CUBE / GROUPING SETS merged groups from different grouping sets + +describe(`Test ${testId} - CUBE and GROUPING SETS keep grouping sets apart`, function () { + const data = [ + {a: 2, b: 2, v: 15}, + {a: 2, b: 4, v: 20}, + ]; + + it('A) CUBE(a, b) from the issue', function () { + const res = alasql('SELECT a, b, SUM(v) AS s FROM ? GROUP BY CUBE(a, b)', [data]); + assert.deepStrictEqual(res, [ + {a: null, b: null, s: 35}, + {a: 2, b: null, s: 35}, + {a: null, b: 2, s: 15}, + {a: 2, b: 2, s: 15}, + {a: null, b: 4, s: 20}, + {a: 2, b: 4, s: 20}, + ]); + }); + + it('B) GROUPING SETS(a, b) with overlapping values', function () { + const res = alasql('SELECT a, b, SUM(v) AS s FROM ? GROUP BY GROUPING SETS(a, b)', [data]); + assert.deepStrictEqual(res, [ + {a: 2, b: null, s: 35}, + {a: null, b: 2, s: 15}, + {a: null, b: 4, s: 20}, + ]); + }); + + it('C) CUBE with COUNT over several rows', function () { + const rows = [ + {x: 'p', y: 'p'}, + {x: 'p', y: 'q'}, + {x: 'q', y: 'p'}, + ]; + const res = alasql('SELECT x, y, COUNT(*) AS c FROM ? GROUP BY CUBE(x, y)', [rows]); + assert.deepStrictEqual(res, [ + {x: null, y: null, c: 3}, + {x: 'p', y: null, c: 2}, + {x: null, y: 'p', c: 2}, + {x: 'p', y: 'p', c: 1}, + {x: null, y: 'q', c: 1}, + {x: 'p', y: 'q', c: 1}, + {x: 'q', y: null, c: 1}, + {x: 'q', y: 'p', c: 1}, + ]); + }); + + it('D) ROLLUP and plain GROUP BY are unchanged', function () { + assert.deepStrictEqual( + alasql('SELECT a, b, SUM(v) AS s FROM ? GROUP BY ROLLUP(a, b)', [data]), + [ + {a: null, b: null, s: 35}, + {a: 2, b: null, s: 35}, + {a: 2, b: 2, s: 15}, + {a: 2, b: 4, s: 20}, + ] + ); + assert.deepStrictEqual(alasql('SELECT a, b, SUM(v) AS s FROM ? GROUP BY a, b', [data]), [ + {a: 2, b: 2, s: 15}, + {a: 2, b: 4, s: 20}, + ]); + }); +});