Add GROUP_CONCAT support for ORDER BY and SEPARATOR clauses #2418

Copilot · 2025-12-26T12:09:45Z

Summary of Implementation

This PR implements full MySQL-style GROUP_CONCAT syntax support with ORDER BY and SEPARATOR clauses, resolving the parse error that prevented users from using these features.

Parser Implementation

GROUP_CONCAT as First-Class Token:

Added GROUP_CONCAT as a lexer token alongside other SQL keywords (SUM, COUNT, AVG, etc.)
Integrated GROUP_CONCAT into the Aggregator rule for consistent treatment
Implemented proper grammar rules using GROUP_CONCAT LPAR ... RPAR syntax instead of runtime IF checks
Added helper rules GroupConcatOrderClause and GroupConcatSeparatorClause for clean grammar structure

Proper SEPARATOR Token Handling (Jison Best Practice):

Added SEPARATOR as a keyword token in the lexer
Added SEPARATOR to the NonReserved grammar rule for context-aware usage
Added SEPARATOR to the nonReserved JavaScript array (line 3705)
Added SEPARATOR to the Literal rule so it can be used as an identifier in contexts like JSON property names
Uses SEPARATOR STRING in GroupConcatSeparatorClause for clean grammar without runtime checks
This follows the proper Jison pattern: define as keyword, mark as non-reserved (both in grammar and JS array), allow in Literal contexts
Added processing for common escape sequences in SEPARATOR strings (\n, \t, \r, \)

Aggregator Implementation

Enhanced GROUP_CONCAT Function (src/55functions.js):

Uses array-based accumulator for collecting values
Implements sorting based on ORDER BY direction (ASC/DESC)
Supports custom separators (default remains comma)
Maintains backwards compatibility with existing string-based format

Compilation Optimization (src/423groupby.js):

Created getGroupConcatParams() helper function to eliminate code duplication
Helper used in both stage 1 and stage 2 compilation
Reduces implementation from ~40 lines to ~5 lines of logic
Extracts order direction and separator at compile time

Supported Syntax

-- Basic usage (existing)
SELECT GROUP_CONCAT(column) FROM table;

-- With DISTINCT
SELECT GROUP_CONCAT(DISTINCT column) FROM table;

-- With ORDER BY
SELECT GROUP_CONCAT(column ORDER BY column ASC) FROM table;
SELECT GROUP_CONCAT(column ORDER BY column DESC) FROM table;

-- With custom SEPARATOR
SELECT GROUP_CONCAT(column SEPARATOR ';') FROM table;
SELECT GROUP_CONCAT(column SEPARATOR '\n') FROM table;

-- Full syntax with all options
SELECT GROUP_CONCAT(DISTINCT column ORDER BY column DESC SEPARATOR ';') FROM table;

Testing

✅ All 12 GROUP_CONCAT tests passing (test2413.js, test623.js, test1898.js)
✅ All CSV/TSV tests passing (test176.js) - SEPARATOR works in both SQL and JSON contexts
✅ Comprehensive test coverage for all feature combinations
✅ Backwards compatibility verified with existing tests
✅ Tests use assert.deepEqual for complete object validation (except test B which tests unordered results)

The implementation follows proper Jison/yacc parser design patterns where keywords can be context-aware by:

Defining them as tokens
Adding them to NonReserved grammar rule
Adding them to nonReserved JavaScript array
Including them in the Literal rule for identifier contexts

This allows SEPARATOR to be a keyword in GROUP_CONCAT while still being usable as a property name in JSON objects.

Original prompt

This section details on the original issue you should resolve

<issue_title>GROUP_CONCAT isn't fully supported</issue_title>
<issue_description>I'm testing the following operation on my app
SELECT GROUP_CONCAT(
    DISTINCT country  
    ORDER BY country ASC SEPARATOR ';'
) as countries
FROM diary;
and it leads to an error: error: Parse error on line 4: ...TINCT country ORDER BY country AS ----------------------^ Expecting 'EOF', 'COMMA', 'RPAR', 'IN', 'LIKE', 'ARROW', 'DOT', 'CARET', 'EQ', 'SLASH', 'EXCLAMATION', 'MODULO', 'GT', 'LT', 'GTGT', 'LTLT', 'NOT', 'AND', 'OR', 'PLUS', 'STAR', 'RBRA', 'END', 'ELSE', 'REGEXP', 'TILDA', 'GLOB', 'NOT_LIKE', 'BARBAR', 'MINUS', 'AMPERSAND', 'BAR', 'GE', 'LE', 'EQEQ', 'EQEQEQ', 'NE', 'NEEQEQ', 'NEEQEQEQ', 'BETWEEN', 'NOT_BETWEEN', 'IS', 'DOUBLECOLON', 'GO', 'SEMICOLON', got 'ORDER'

Do you plan supporting it?

P.S.: In the meanwhile I've solved it by using
SELECT REPLACE(GROUP_CONCAT(country), ',', ';') AS countries
FROM (
    SELECT DISTINCT country
    FROM diary
    ORDER BY country
);
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

Fixes GROUP_CONCAT isn't fully supported #2413

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

mathiasrw · 2025-12-30T13:35:31Z

@copilot Continue

mathiasrw · 2025-12-31T14:05:26Z

@copilot continue

mathiasrw · 2026-01-02T09:32:44Z

Continue

mathiasrw · 2026-01-02T09:35:45Z

@copilot Continue...

Copilot · 2026-01-02T09:54:45Z

Continue

Implementation complete! I've successfully added full support for MySQL-style GROUP_CONCAT with ORDER BY and SEPARATOR clauses.

Changes made (commit 19b2b5c):

✅ Parser now supports ORDER BY and SEPARATOR within GROUP_CONCAT
✅ Updated GROUP_CONCAT aggregator to handle sorting and custom separators
✅ All 8 comprehensive tests passing
✅ Backwards compatible with existing GROUP_CONCAT usage
✅ Code reviewed and security checked

Supported syntax:

SELECT GROUP_CONCAT(DISTINCT country ORDER BY country ASC SEPARATOR ';') 
FROM diary;

@copilot Continue...

Work completed! The GROUP_CONCAT feature is now fully implemented with ORDER BY and SEPARATOR support. All tests are passing and the code has been reviewed for quality and security. The implementation is ready for your review. ✨

mathiasrw

This seems like a lot of code for such a simple issue.

Can you please review and see how we can get a more elegant solution?

mathiasrw · 2026-01-02T10:02:47Z

test/test2413.js

For any test, please do a deepequal to the complete object you are expecting outputted

Tests updated to use assert.deepEqual with complete expected objects (commit 8e65c1f).

Why did you skip test B? Are you missing to update somewhere else?

@copilot Why did you skip test B? Are you missing to update somewhere else?

Test B wasn't updated to use deepEqual because with DISTINCT and no ORDER BY, the result order is non-deterministic. The test correctly validates that:

Exactly 3 unique values are returned

All expected countries are present

Using deepEqual would require a specific order, which would make the test brittle since the order isn't guaranteed by the SQL semantics.

mathiasrw · 2026-01-02T14:24:14Z

src/alasqlparser.jison

+		  if($1.toUpperCase() === 'GROUP_CONCAT') {
+		  	$$ = new yy.AggrValue({aggregatorid: 'REDUCE', funcid: $1, expression: $3, order: $4, separator: $5});
+		  } else {
+		  	throw new Error('Syntax error: ORDER BY and SEPARATOR are only supported in GROUP_CONCAT');
+		  }
+		}
+	| Literal LPAR DISTINCT Expression GroupConcatOrderClause GroupConcatSeparatorClause RPAR
+		{
+		  if($1.toUpperCase() === 'GROUP_CONCAT') {
+		  	$$ = new yy.AggrValue({aggregatorid: 'REDUCE', funcid: $1, expression: $4, distinct: true, order: $5, separator: $6});
+		  } else {
+		  	throw new Error('Syntax error: ORDER BY and SEPARATOR are only supported in GROUP_CONCAT');


Can we please make group_concat its own token instead of starting to do IFs in the grammar?

Refactored to make GROUP_CONCAT a proper token (commit 1688c85):

Added GROUP_CONCAT as a lexer token

Added to Aggregator rule alongside SUM, COUNT, etc.

Removed IF statements from grammar

Now uses proper GROUP_CONCAT LPAR ... RPAR grammar rules

Also added escape sequence processing (\n, \t, \r, \) for SEPARATOR strings.

Co-authored-by: mathiasrw <[email protected]>

…g comments Co-authored-by: mathiasrw <[email protected]>

Co-authored-by: mathiasrw <[email protected]>

…r function Co-authored-by: mathiasrw <[email protected]>

…mmar Co-authored-by: mathiasrw <[email protected]>

mathiasrw · 2026-01-02T15:46:51Z

@copilot

....................................................
.........!

563 passing (781ms)
6 pending
1 failing

Test 176 - CSV and TSV
4. CSV on TAB:
SyntaxError: Parse error on line 1:
...test/test176a.tab",{separator:" ",header
-----------------------^
Expecting 'LITERAL', 'BRALITERAL', 'KEY', 'OPEN', 'NUMBER', 'STRING', 'RCUR', got 'SEPARATOR'
at parser.parseError (dist/alasql.fs.js:2694:8)
at Parser.parse (dist/alasql.fs.js:2297:22)
at alasql.parse (dist/alasql.fs.js:5007:22)

Co-authored-by: mathiasrw <[email protected]>

Copilot · 2026-01-02T15:54:33Z

@copilot

....................................................
...

Fixed (commit e33dc5b). The issue was that SEPARATOR as a reserved token was being recognized in JSON property names like {separator:","}. Changed the implementation to use Literal STRING matching instead, which validates "separator" as a literal string rather than a reserved keyword. All tests now passing including test176.

mathiasrw · 2026-01-02T16:02:21Z

test/test2413.js

Why did you skip test B? Are you missing to update somewhere else?

mathiasrw · 2026-01-02T16:13:59Z

test/test2413.js

src/alasqlparser.jison

Co-authored-by: mathiasrw <[email protected]>

mathiasrw · 2026-01-02T17:02:05Z

src/alasqlparser.jison

 	|SECURITY
 	|SELECTIVE
 	|SELF
+	|SEPARATOR


Did we forget to add it to the list of var nonReserved = ["A" ?

Added (commit 79616ae). SEPARATOR is now in both the NonReserved grammar rule (line 3427) and the nonReserved JavaScript array (line 3705).

mathiasrw · 2026-01-02T17:02:27Z

test/test2413.js

@copilot Why did you skip test B? Are you missing to update somewhere else?

Co-authored-by: mathiasrw <[email protected]>

Copilot AI assigned Copilot and mathiasrw Dec 26, 2025

Copilot AI requested a review from mathiasrw December 26, 2025 12:09

Copilot started work on behalf of mathiasrw January 2, 2026 09:36 View session

Copilot AI changed the title ~~[WIP] Fix GROUP_CONCAT support in SQL queries~~ Add GROUP_CONCAT support for ORDER BY and SEPARATOR clauses Jan 2, 2026

Copilot finished work on behalf of mathiasrw January 2, 2026 09:55

mathiasrw requested changes Jan 2, 2026

View reviewed changes

Copilot started work on behalf of mathiasrw January 2, 2026 10:07 View session

Copilot AI requested a review from mathiasrw January 2, 2026 10:11

Copilot started work on behalf of mathiasrw January 2, 2026 14:13 View session

Copilot finished work on behalf of mathiasrw January 2, 2026 14:17

mathiasrw requested changes Jan 2, 2026

View reviewed changes

Copilot started work on behalf of mathiasrw January 2, 2026 14:24 View session

Copilot AI requested a review from mathiasrw January 2, 2026 14:31

Copilot finished work on behalf of mathiasrw January 2, 2026 14:31

mathiasrw approved these changes Jan 2, 2026

View reviewed changes

Copilot AI and others added 5 commits January 3, 2026 02:18

Initial plan

e18e9a2

Add test for GROUP_CONCAT with ORDER BY and SEPARATOR

8b64628

Co-authored-by: mathiasrw <[email protected]>

Implement GROUP_CONCAT with ORDER BY and SEPARATOR support

f9f8e36

Co-authored-by: mathiasrw <[email protected]>

Address code review feedback - fix comparison logic and add clarifyin…

9a87fb7

…g comments Co-authored-by: mathiasrw <[email protected]>

Changes before error encountered

591aa43

Co-authored-by: mathiasrw <[email protected]>

Copilot AI and others added 4 commits January 3, 2026 02:18

Complete refactoring: simplify GROUP_CONCAT implementation with helpe…

9d24285

…r function Co-authored-by: mathiasrw <[email protected]>

Change separator in SQL query from ';' to '\n'

3cc3100

Refactor: Make GROUP_CONCAT a proper token instead of using IF in gra…

1201ddd

…mmar Co-authored-by: mathiasrw <[email protected]>

Merge

165f40e

mathiasrw force-pushed the copilot/fix-group-concat-support branch from 1688c85 to 165f40e Compare January 2, 2026 15:19

Copilot started work on behalf of mathiasrw January 2, 2026 15:47 View session

Fix: Remove SEPARATOR as reserved token to allow it in JSON properties

e33dc5b

Co-authored-by: mathiasrw <[email protected]>

Copilot finished work on behalf of mathiasrw January 2, 2026 15:55

Copilot AI requested a review from mathiasrw January 2, 2026 15:55

mathiasrw requested changes Jan 2, 2026

View reviewed changes

test/test2413.js

Copy link

Member

mathiasrw Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you skip test B? Are you missing to update somewhere else?

Copilot started work on behalf of mathiasrw January 2, 2026 16:02 View session

Copilot finished work on behalf of mathiasrw January 2, 2026 16:04

Copilot AI requested a review from mathiasrw January 2, 2026 16:04

mathiasrw requested changes Jan 2, 2026

View reviewed changes

test/test2413.js

Copy link

Member

mathiasrw Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot

src/alasqlparser.jison Show resolved Hide resolved

Copilot started work on behalf of mathiasrw January 2, 2026 16:15 View session

Refactor: Add SEPARATOR as keyword token with NonReserved status

179d3bd

Co-authored-by: mathiasrw <[email protected]>

Copilot AI requested a review from mathiasrw January 2, 2026 16:21

Copilot finished work on behalf of mathiasrw January 2, 2026 16:21

mathiasrw requested changes Jan 2, 2026

View reviewed changes

Copilot started work on behalf of mathiasrw January 2, 2026 17:03 View session

Add SEPARATOR to nonReserved JavaScript array

79616ae

Co-authored-by: mathiasrw <[email protected]>

Copilot AI requested a review from mathiasrw January 2, 2026 17:07

Copilot finished work on behalf of mathiasrw January 2, 2026 17:07

Add GROUP_CONCAT support for ORDER BY and SEPARATOR clauses #2418

Are you sure you want to change the base?

Add GROUP_CONCAT support for ORDER BY and SEPARATOR clauses #2418

Conversation

Copilot AI commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Implementation

Parser Implementation

Aggregator Implementation

Supported Syntax

Testing

Uh oh!

mathiasrw commented Dec 30, 2025

Uh oh!

mathiasrw commented Dec 31, 2025

Uh oh!

mathiasrw commented Jan 2, 2026

Uh oh!

mathiasrw commented Jan 2, 2026

Uh oh!

Copilot AI commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mathiasrw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

mathiasrw commented Jan 2, 2026

Uh oh!

Copilot AI commented Jan 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Dec 26, 2025 •

edited

Loading

Copilot AI commented Jan 2, 2026 •

edited

Loading