Skip to content

Commit c6b6e20

Browse files
Copilotmathiasrw
Copilot
and
mathiasrw
authored
Add SEPARATOR clauses and fix ORDER BY for GROUP_CONCAT to close #2413 (#2418)
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 e4ad005 commit c6b6e20

File tree

9 files changed

+1068
-835
lines changed

9 files changed

+1068
-835
lines changed

‎.github/copilot-instructions.md‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ AlaSQL is an open source SQL database for JavaScript with a focus on query speed
77
## When Implementing Features
88

99
1. **Understand the issue thoroughly** - Read related test cases and existing code
10-
2. **Write a test first** - Copy test/test000.js into a new file called `test/test###.js` where where `###` is the id of the issue we are trying to solve
10+
2. **Write a test first** - Copy test/test000.js into a new file called `test/test###.js` where where `###` is the id of the issue we are trying to solve.
1111
3. **Verify test fails** - Run `yarn test` to confirm the test catches the issue
1212
4. **Implement the fix** - Modify appropriate file(s) in `src/`
1313
- If you modify the grammar in `src/alasqlgrammar.jison`, run `yarn jison && yarn test` to regenerate the parser and verify
@@ -37,3 +37,12 @@ yarn format
3737
- `src/alasqlparser.js` - Generated from Jison grammar (modify the `.jison` file instead)
3838
- `.min.js` files - Generated during build
3939

40+
41+
## Plesae note
42+
43+
- Alasql is meant to return `undefined` instead of `null` (unline regular SQL engines)
44+
45+
## Resources
46+
47+
- [AlaSQL Documentation](https://github.com/alasql/alasql/wiki)
48+
- [Issue Tracker](https://github.com/AlaSQL/alasql/issues)

‎src/423groupby.js‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,24 @@
66
//
77
*/
88

9+
/**
10+
* Helper to get GROUP_CONCAT parameters for compilation
11+
* @param {Object} col - Column with aggregatorid and funcid
12+
* @returns {string} - Comma-separated parameter string for aggregator call
13+
*/
14+
function getGroupConcatParams(col) {
15+
if (!col.funcid || col.funcid.toUpperCase() !== 'GROUP_CONCAT') {
16+
return '';
17+
}
18+
const separator = col.separator !== undefined ? JSON.stringify(col.separator) : 'undefined';
19+
// Extract direction from order expressions (MySQL GROUP_CONCAT orders by the value itself)
20+
const orderDir =
21+
col.order && col.order.length > 0 && col.order[0].direction
22+
? JSON.stringify(col.order[0].direction)
23+
: 'undefined';
24+
return `,${separator},${orderDir}`;
25+
}
26+
927
/**
1028
Compile group of statements
1129
*/
@@ -117,15 +135,15 @@ yy.Select.prototype.compileGroup = function (query) {
117135
if ('funcid' in col.expression) {
118136
let colexp1 = colExpIfFunIdExists(col.expression);
119137

120-
return `'${colas}': (__alasql_tmp = ${colexp}, typeof __alasql_tmp == 'number' || typeof __alasql_tmp == 'bigint' || (typeof __alasql_tmp == 'object' && (typeof Number(__alasql_tmp) == 'number' || __alasql_tmp instanceof Date)) ? __alasql_tmp : undefined),`;
138+
return `'${colas}': (__alasql_tmp = ${colexp}, __alasql_tmp !== null && (typeof __alasql_tmp == 'number' || typeof __alasql_tmp == 'bigint' || (typeof __alasql_tmp == 'object' && (typeof Number(__alasql_tmp) == 'number' || __alasql_tmp instanceof Date))) ? __alasql_tmp : undefined),`;
121139
}
122-
return `'${colas}': (__alasql_tmp = ${colexp}, typeof __alasql_tmp == 'number' || typeof __alasql_tmp == 'bigint' || (typeof __alasql_tmp == 'object' && (typeof Number(__alasql_tmp) == 'number' || __alasql_tmp instanceof Date)) ? __alasql_tmp : undefined),`;
140+
return `'${colas}': (__alasql_tmp = ${colexp}, __alasql_tmp !== null && (typeof __alasql_tmp == 'number' || typeof __alasql_tmp == 'bigint' || (typeof __alasql_tmp == 'object' && (typeof Number(__alasql_tmp) == 'number' || __alasql_tmp instanceof Date))) ? __alasql_tmp : undefined),`;
123141
} else if (col.aggregatorid === 'MAX') {
124142
if ('funcid' in col.expression) {
125143
let colexp1 = colExpIfFunIdExists(col.expression);
126-
return `'${colas}': (__alasql_tmp = ${colexp}, typeof __alasql_tmp == 'number' || typeof __alasql_tmp == 'bigint' || (typeof __alasql_tmp == 'object' && (typeof Number(__alasql_tmp) == 'number' || __alasql_tmp instanceof Date)) ? __alasql_tmp : undefined),`;
144+
return `'${colas}': (__alasql_tmp = ${colexp}, __alasql_tmp !== null && (typeof __alasql_tmp == 'number' || typeof __alasql_tmp == 'bigint' || (typeof __alasql_tmp == 'object' && (typeof Number(__alasql_tmp) == 'number' || __alasql_tmp instanceof Date))) ? __alasql_tmp : undefined),`;
127145
}
128-
return `'${colas}' : (typeof ${colexp} == 'number' || typeof ${colexp} == 'bigint' ? ${colexp} : typeof ${colexp} == 'object' ?
146+
return `'${colas}' : (${colexp} !== null && (typeof ${colexp} == 'number' || typeof ${colexp} == 'bigint') ? ${colexp} : ${colexp} !== null && typeof ${colexp} == 'object' ?
129147
typeof Number(${colexp}) == 'number' ? ${colexp} : undefined : undefined),`;
130148
} else if (col.aggregatorid === 'ARRAY') {
131149
return `'${colas}':[${colexp}],`;
@@ -145,7 +163,8 @@ yy.Select.prototype.compileGroup = function (query) {
145163
return '';
146164
} else if (col.aggregatorid === 'REDUCE') {
147165
query.aggrKeys.push(col);
148-
return `'${colas}':alasql.aggr['${col.funcid}'](${colexp},undefined,1),`;
166+
const extraParams = getGroupConcatParams(col);
167+
return `'${colas}':alasql.aggr['${col.funcid}'](${colexp},undefined,1${extraParams}),`;
149168
}
150169
return '';
151170
}
@@ -415,8 +434,9 @@ yy.Select.prototype.compileGroup = function (query) {
415434
g['${colas}']=${col.expression.toJS('g', -1)};
416435
${post}`;
417436
} else if (col.aggregatorid === 'REDUCE') {
437+
const extraParams = getGroupConcatParams(col);
418438
return `${pre}
419-
g['${colas}'] = alasql.aggr.${col.funcid}(${colexp},g['${colas}'],2);
439+
g['${colas}'] = alasql.aggr.${col.funcid}(${colexp},g['${colas}'],2${extraParams});
420440
${post}`;
421441
}
422442

‎src/55functions.js‎

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -281,26 +281,76 @@ stdfn.CONCAT_WS = function () {
281281
// TRIM
282282

283283
// Aggregator for joining strings
284-
alasql.aggr.group_concat = alasql.aggr.GROUP_CONCAT = function (v, s, stage) {
284+
// Can be called with options: alasql.aggr.GROUP_CONCAT(v, s, stage, separator, orderDirection)
285+
alasql.aggr.group_concat = alasql.aggr.GROUP_CONCAT = function (
286+
v,
287+
s,
288+
stage,
289+
separator,
290+
orderDirection
291+
) {
292+
// Default separator is comma
293+
if (separator === undefined) {
294+
separator = ',';
295+
}
296+
285297
if (stage === 1) {
286-
// Initialize: skip null values
298+
// Initialize: create array to collect values
299+
// Store as object with values array and metadata
287300
if (v === null || v === undefined) {
288-
return null;
301+
return {values: [], separator: separator, orderDirection: orderDirection};
289302
}
290-
return '' + v;
303+
return {values: [v], separator: separator, orderDirection: orderDirection};
291304
} else if (stage === 2) {
292-
// Accumulate: skip null values
305+
// Accumulate: add to values array, skip null values
293306
if (v === null || v === undefined) {
294307
return s;
295308
}
296-
// If accumulator is null/undefined, start with current value
309+
// If accumulator is null/undefined, initialize it
297310
if (s === null || s === undefined) {
298-
return '' + v;
311+
return {values: [v], separator: separator, orderDirection: orderDirection};
299312
}
300-
s += ',' + v;
313+
// Handle both old string format (for backwards compatibility) and new object format
314+
if (typeof s === 'string') {
315+
// Old format - convert to new format
316+
s = {values: s.split(','), separator: ',', orderDirection: undefined};
317+
}
318+
s.values.push(v);
301319
return s;
320+
} else {
321+
// Stage 3 (or final): sort if needed and join
322+
if (s === null || s === undefined) {
323+
return undefined;
324+
}
325+
// Handle both old string format and new object format
326+
if (typeof s === 'string') {
327+
return s; // Already formatted
328+
}
329+
330+
let values = s.values;
331+
332+
// If no values were collected (all nulls), return undefined
333+
if (values.length === 0) {
334+
return undefined;
335+
}
336+
337+
// Sort if orderDirection is provided (check for actual undefined, not the string 'undefined')
338+
if (s.orderDirection && s.orderDirection !== undefined) {
339+
let ascending = s.orderDirection === 'ASC';
340+
values = values.slice().sort((a, b) => {
341+
if (a === b) return 0;
342+
if (a === null || a === undefined) return 1;
343+
if (b === null || b === undefined) return -1;
344+
if (typeof a === 'string' && typeof b === 'string') {
345+
return ascending ? a.localeCompare(b) : b.localeCompare(a);
346+
}
347+
// For numbers and other types - add parentheses for clarity
348+
return ascending ? (a < b ? -1 : 1) : b < a ? -1 : 1;
349+
});
350+
}
351+
352+
return values.join(s.separator);
302353
}
303-
return s;
304354
};
305355

306356
alasql.aggr.median = alasql.aggr.MEDIAN = function (v, s, stage) {
@@ -319,7 +369,7 @@ alasql.aggr.median = alasql.aggr.MEDIAN = function (v, s, stage) {
319369
}
320370

321371
if (!s.length) {
322-
return null;
372+
return undefined;
323373
}
324374

325375
let r = s.sort((a, b) => {

‎src/84from.js‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,16 @@ alasql.from.CSV = function (contents, opts, cb, idx, query) {
256256
alasql.utils.extend(opt, opts);
257257
var res;
258258
var hs = [];
259+
// Determine once whether to auto-convert: not raw mode and not SELECT INTO
260+
const shouldAutoConvert = !opt.raw && !query?.intofns;
261+
262+
function potentialAutoConvert(val) {
263+
if (shouldAutoConvert && val !== undefined && val.length !== 0 && val == +val) {
264+
return +val;
265+
}
266+
return val;
267+
}
268+
259269
function parseText(text) {
260270
var delimiterCode = opt.separator.charCodeAt(0);
261271
var quoteCode = opt.quote.charCodeAt(0);
@@ -332,25 +342,22 @@ alasql.from.CSV = function (contents, opts, cb, idx, query) {
332342
hs = opt.headers;
333343
var r = {};
334344
hs.forEach(function (h, idx) {
335-
r[h] = a[idx];
336-
// Keep as string - type conversion happens at INSERT time based on column definitions
345+
r[h] = potentialAutoConvert(a[idx]);
337346
});
338347
rows.push(r);
339348
}
340349
} else {
341350
var r = {};
342351
hs.forEach(function (h, idx) {
343-
r[h] = a[idx];
344-
// Keep as string - type conversion happens at INSERT time based on column definitions
352+
r[h] = potentialAutoConvert(a[idx]);
345353
});
346354
rows.push(r);
347355
}
348356
n++;
349357
} else {
350358
var r = {};
351-
// Keep as string - type conversion happens at INSERT time based on column definitions
352359
a.forEach(function (v, idx) {
353-
r[idx] = a[idx];
360+
r[idx] = potentialAutoConvert(a[idx]);
354361
});
355362
rows.push(r);
356363
}

‎src/alasqlparser.jison‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ DATABASE(S)? return 'DATABASE'
144144
'GO' return 'GO'
145145
'GRAPH' return 'GRAPH'
146146
'GROUP' return 'GROUP'
147+
'GROUP_CONCAT' return 'GROUP_CONCAT'
147148
'GROUPING' return 'GROUPING'
148149
'HAVING' return 'HAVING'
149150
/*'HELP' return 'HELP'*/
@@ -233,6 +234,7 @@ SCHEMA(S)? return 'DATABASE'
233234
'SEARCH' return 'SEARCH'
234235

235236
'SEMI' return 'SEMI'
237+
'SEPARATOR' return 'SEPARATOR'
236238
SET return 'SET'
237239
SETS return 'SET'
238240
'SHOW' return 'SHOW'
@@ -365,6 +367,8 @@ Literal
365367
{ $$ = $1.toLowerCase(); }
366368
| CLOSE
367369
{ $$ = $1.toLowerCase(); }
370+
| SEPARATOR
371+
{ $$ = $1.toLowerCase(); }
368372
| error NonReserved
369373
{ $$ = $2.toLowerCase() }
370374
;
@@ -1406,6 +1410,10 @@ AggrValue
14061410
| Aggregator LPAR ALL Expression RPAR OverClause
14071411
{ $$ = new yy.AggrValue({aggregatorid: $1.toUpperCase(), expression: $4,
14081412
over:$6}); }
1413+
| GROUP_CONCAT LPAR Expression GroupConcatOrderClause GroupConcatSeparatorClause RPAR
1414+
{ $$ = new yy.AggrValue({aggregatorid: 'REDUCE', funcid: 'GROUP_CONCAT', expression: $3, order: $4, separator: $5}); }
1415+
| GROUP_CONCAT LPAR DISTINCT Expression GroupConcatOrderClause GroupConcatSeparatorClause RPAR
1416+
{ $$ = new yy.AggrValue({aggregatorid: 'REDUCE', funcid: 'GROUP_CONCAT', expression: $4, distinct: true, order: $5, separator: $6}); }
14091417
;
14101418

14111419
OverClause
@@ -1422,6 +1430,26 @@ OverOrderByClause
14221430
: ORDER BY OrderExpressionsList
14231431
{ $$ = {order:$3}; }
14241432
;
1433+
1434+
GroupConcatOrderClause
1435+
:
1436+
{ $$ = undefined; }
1437+
| ORDER BY OrderExpressionsList
1438+
{ $$ = $3; }
1439+
;
1440+
1441+
GroupConcatSeparatorClause
1442+
:
1443+
{ $$ = undefined; }
1444+
| SEPARATOR STRING
1445+
{
1446+
var str = $2.substring(1, $2.length-1);
1447+
// Process common escape sequences
1448+
str = str.replace(/\\n/g, '\n').replace(/\\t/g, '\t').replace(/\\r/g, '\r').replace(/\\\\/g, '\\');
1449+
$$ = str;
1450+
}
1451+
;
1452+
14251453
Aggregator
14261454
: SUM { $$ = "SUM"; }
14271455
| TOTAL { $$ = "TOTAL"; }
@@ -1433,6 +1461,7 @@ Aggregator
14331461
| LAST { $$ = "LAST"; }
14341462
| AGGR { $$ = "AGGR"; }
14351463
| ARRAY { $$ = "ARRAY"; }
1464+
| GROUP_CONCAT { $$ = "GROUP_CONCAT"; }
14361465
/* | REDUCE { $$ = "REDUCE"; } */
14371466
;
14381467

@@ -3380,6 +3409,7 @@ NonReserved
33803409
|SECURITY
33813410
|SELECTIVE
33823411
|SELF
3412+
|SEPARATOR
33833413
|SEQUENCE
33843414
|SERIALIZABLE
33853415
|SERVER
@@ -3658,6 +3688,7 @@ var nonReserved = ["A"
36583688
,"SECURITY"
36593689
,"SELECTIVE"
36603690
,"SELF"
3691+
,"SEPARATOR"
36613692
,"SEQUENCE"
36623693
,"SERIALIZABLE"
36633694
,"SERVER"

‎src/alasqlparser.js‎

Lines changed: 830 additions & 808 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)