Repository: paustint/soql-parser-js (ts)

Issue:
Group by does not allow multiple fields
<!--
  Please include the following classification.
  If your classification is not added, you can modify the list below
-->
- [ ] Feature
- [x] Bug Request

<!--If this is a bug, please include the steps to reproduce the bug-->
## Description

The following query is listed as invalid, but it is valid syntax.

```sql
SELECT BillingState, BillingStreet, COUNT(Id)
FROM Account
GROUP BY BillingState, BillingStreet
```

Expected output (based on current data model):
```typescript
      fields: [
        { type: 'Field', field: 'BillingState' },
        { type: 'Field', field: 'BillingStreet' },
        {
          type: 'FieldFunctionExpression',
          functionName: 'COUNT',
          rawValue: 'COUNT(Id)',
          isAggregateFn: true,
          parameters: ['Id'],
        },
      ],
      sObject: 'Account',
      groupBy: { field: ['BillingState', 'BillingStreet'] },
```


Pull request:
Group by does not allow multiple fields #85
Fixed bug to allow Group By clause to parse multiple fields

resolves #85

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a8a6f91..a94e946 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## 2.2.1
+
+Nov 17, 2019
+
+1. `GROUP BY` did not allow multiple fields to be listed, for example: `SELECT BillingState, BillingStreet, COUNT(Id) FROM Account GROUP BY BillingState, BillingStreet` would fail to parse.
+
 ## 2.2.0
 
 Nov 6, 2019
diff --git a/src/models.ts b/src/models.ts
index 644bfd7..286fa11 100644
--- a/src/models.ts
+++ b/src/models.ts
@@ -3,8 +3,6 @@ import { LiteralType } from './api/api-models';
 
 export type LiteralTypeWithSubquery = (LiteralType | 'SUBQUERY') | LiteralType[];
 
-// export type HavingConditionWithDateLiteralVar = HavingCondition & { dateLiteralVariable: string };
-
 export interface ArrayExpressionWithType {
   type: string;
   value: string;
@@ -108,11 +106,16 @@ export interface WithDateCategoryConditionContext {
   dataCategoryName: IToken[];
 }
 
-export interface GroupByClauseContext extends WithIdentifier {
+export interface GroupByClauseContext {
+  groupByFieldList?: CstNode[];
   fn: CstNode[];
   havingClause: CstNode[];
 }
 
+export interface GroupByFieldListContext {
+  field: IToken[];
+}
+
 export interface HavingClauseContext {
   conditionExpression: CstNode[];
 }
@@ -232,5 +235,3 @@ export interface ExpressionWithAggregateFunctionContext {
   relationalOperator?: CstNode[];
   setOperator?: CstNode[];
 }
-
-// CstNode | CstNode[]
diff --git a/src/parser/parser.ts b/src/parser/parser.ts
index 2c84044..c169612 100644
--- a/src/parser/parser.ts
+++ b/src/parser/parser.ts
@@ -283,13 +283,22 @@ export class SoqlParser extends CstParser {
       { ALT: () => this.SUBRULE(this.cubeFunction, { LABEL: 'fn' }) },
       { ALT: () => this.SUBRULE(this.rollupFunction, { LABEL: 'fn' }) },
       { ALT: () => this.SUBRULE(this.dateFunction, { LABEL: 'fn' }) },
-      { ALT: () => this.CONSUME1(lexer.Identifier) },
+      { ALT: () => this.SUBRULE(this.groupByFieldList) },
     ]);
     this.OPTION(() => {
       this.SUBRULE(this.havingClause);
     });
   });
 
+  private groupByFieldList = this.RULE('groupByFieldList', () => {
+    this.AT_LEAST_ONE_SEP({
+      SEP: lexer.Comma,
+      DEF: () => {
+        this.CONSUME(lexer.Identifier, { LABEL: 'field' });
+      },
+    });
+  });
+
   private havingClause = this.RULE('havingClause', () => {
     this.CONSUME(lexer.Having);
 
diff --git a/src/parser/visitor.ts b/src/parser/visitor.ts
index b4d5f1c..d0583d1 100644
--- a/src/parser/visitor.ts
+++ b/src/parser/visitor.ts
@@ -58,6 +58,7 @@ import {
   LocationFunctionContext,
   GeoLocationFunctionContext,
   OrderByLocationExpressionContext,
+  GroupByFieldListContext,
 } from '../models';
 import { isSubqueryFromFlag, isToken } from '../utils';
 import { parse, ParseQueryConfig, SoqlParser } from './parser';
@@ -365,7 +366,7 @@ class SOQLVisitor extends BaseSoqlVisitor {
   }
 
   groupByClause(ctx: GroupByClauseContext): GroupByClause {
-    let field = ctx.Identifier ? ctx.Identifier.map((item: any) => item.image) : undefined;
+    let field = ctx.groupByFieldList ? ctx.groupByFieldList.map((item: any) => this.visit(item)) : undefined;
     if (field && field.length === 1) {
       field = field[0];
     }
@@ -382,6 +383,14 @@ class SOQLVisitor extends BaseSoqlVisitor {
     return output;
   }
 
+  groupByFieldList(ctx: GroupByFieldListContext): string | string[] {
+    if (ctx.field.length > 1) {
+      return ctx.field.map((item: any) => item.image);
+    } else {
+      return ctx.field[0].image;
+    }
+  }
+
   havingClause(ctx: HavingClauseContext): HavingClause {
     // expressionWithAggregateFunction
     const having = ctx.conditionExpression.reduce(


Test patch:
diff --git a/test/test-cases.ts b/test/test-cases.ts
index 385a726..5a7ab8a 100644
--- a/test/test-cases.ts
+++ b/test/test-cases.ts
@@ -15,7 +15,11 @@ export const testCases: TestCase[] = [
     testCase: 1,
     soql: 'SELECT Id, Name, BillingCity FROM Account',
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Name' }, { type: 'Field', field: 'BillingCity' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Name' },
+        { type: 'Field', field: 'BillingCity' },
+      ],
       sObject: 'Account',
     },
   },
@@ -114,7 +118,10 @@ export const testCases: TestCase[] = [
     testCase: 9,
     soql: 'SELECT Name, Id FROM Merchandise__c ORDER BY Name OFFSET 100',
     output: {
-      fields: [{ type: 'Field', field: 'Name' }, { type: 'Field', field: 'Id' }],
+      fields: [
+        { type: 'Field', field: 'Name' },
+        { type: 'Field', field: 'Id' },
+      ],
       sObject: 'Merchandise__c',
       orderBy: { field: 'Name' },
       offset: 100,
@@ -124,7 +131,10 @@ export const testCases: TestCase[] = [
     testCase: 10,
     soql: 'SELECT Name, Id FROM Merchandise__c ORDER BY Name LIMIT 20 OFFSET 100',
     output: {
-      fields: [{ type: 'Field', field: 'Name' }, { type: 'Field', field: 'Id' }],
+      fields: [
+        { type: 'Field', field: 'Name' },
+        { type: 'Field', field: 'Id' },
+      ],
       sObject: 'Merchandise__c',
       orderBy: { field: 'Name' },
       limit: 20,
@@ -339,7 +349,13 @@ export const testCases: TestCase[] = [
   {
     testCase: 24,
     soql: 'SELECT UserId, LoginTime FROM LoginHistory',
-    output: { fields: [{ type: 'Field', field: 'UserId' }, { type: 'Field', field: 'LoginTime' }], sObject: 'LoginHistory' },
+    output: {
+      fields: [
+        { type: 'Field', field: 'UserId' },
+        { type: 'Field', field: 'LoginTime' },
+      ],
+      sObject: 'LoginHistory',
+    },
   },
   {
     testCase: 25,
@@ -742,7 +758,10 @@ export const testCases: TestCase[] = [
     testCase: 47,
     soql: "SELECT Id, Name FROM Account WHERE Id IN (SELECT AccountId FROM Opportunity WHERE StageName = 'Closed Lost')",
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Name' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Name' },
+      ],
       sObject: 'Account',
       where: {
         left: {
@@ -781,7 +800,10 @@ export const testCases: TestCase[] = [
     soql:
       "SELECT Id, Name FROM Account WHERE Id IN (SELECT AccountId FROM Contact WHERE LastName LIKE 'apple%') AND Id IN (SELECT AccountId FROM Opportunity WHERE isClosed = FALSE)",
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Name' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Name' },
+      ],
       sObject: 'Account',
       where: {
         left: {
@@ -850,7 +872,10 @@ export const testCases: TestCase[] = [
     soql: "SELECT Id, Name FROM Account WHERE Name != 'foo'",
     soqlComposed: "SELECT Id, Name FROM Account WHERE Name != 'foo'",
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Name' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Name' },
+      ],
       sObject: 'Account',
       where: { left: { field: 'Name', operator: '!=', value: "'foo'", literalType: 'STRING' } },
     },
@@ -880,7 +905,10 @@ export const testCases: TestCase[] = [
     testCase: 54,
     soql: 'SELECT Id, Name FROM Account WHERE CreatedDate > LAST_N_YEARS:1 AND LastModifiedDate > LAST_MONTH',
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Name' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Name' },
+      ],
       sObject: 'Account',
       where: {
         left: {
@@ -921,7 +949,11 @@ export const testCases: TestCase[] = [
     testCase: 56,
     soql: 'SELECT Id, Name, BillingCity FROM Account WITH SECURITY_ENFORCED',
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Name' }, { type: 'Field', field: 'BillingCity' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Name' },
+        { type: 'Field', field: 'BillingCity' },
+      ],
       sObject: 'Account',
       withSecurityEnforced: true,
     },
@@ -1216,7 +1248,10 @@ export const testCases: TestCase[] = [
     testCase: 74,
     soql: "SELECT Id, MSP1__c FROM CustObj__c WHERE MSP1__c INCLUDES ('AAA;BBB', 'CCC')",
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'MSP1__c' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'MSP1__c' },
+      ],
       sObject: 'CustObj__c',
       where: { left: { field: 'MSP1__c', literalType: 'STRING', operator: 'INCLUDES', value: ["'AAA;BBB'", "'CCC'"] } },
     },
@@ -1276,7 +1311,10 @@ export const testCases: TestCase[] = [
     testCase: 78,
     soql: "SELECT Id, Title FROM Dashboard USING SCOPE allPrivate WHERE Type != 'SpecifiedUser'",
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Title' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Title' },
+      ],
       sObject: 'Dashboard',
       usingScope: 'allPrivate',
       where: { left: { field: 'Type', literalType: 'STRING', operator: '!=', value: "'SpecifiedUser'" } },
@@ -1415,7 +1453,10 @@ export const testCases: TestCase[] = [
     testCase: 84,
     soql: 'SELECT Id, Amount FROM Opportunity WHERE Amount IN (usd500.01, usd600)',
     output: {
-      fields: [{ type: 'Field', field: 'Id' }, { type: 'Field', field: 'Amount' }],
+      fields: [
+        { type: 'Field', field: 'Id' },
+        { type: 'Field', field: 'Amount' },
+      ],
       sObject: 'Opportunity',
       where: {
         left: {
@@ -1540,7 +1581,10 @@ export const testCases: TestCase[] = [
     testCase: 88,
     soql: `SELECT Name, Location__c FROM Warehouse__c WHERE DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi') < 20`,
     output: {
-      fields: [{ type: 'Field', field: 'Name' }, { type: 'Field', field: 'Location__c' }],
+      fields: [
+        { type: 'Field', field: 'Name' },
+        { type: 'Field', field: 'Location__c' },
+      ],
       sObject: 'Warehouse__c',
       where: {
         left: {
@@ -1568,7 +1612,10 @@ export const testCases: TestCase[] = [
     testCase: 89,
     soql: `SELECT Name, StreetAddress__c FROM Warehouse__c WHERE DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi') < 20 ORDER BY DISTANCE(Location__c, GEOLOCATION(37.775, -122.418), 'mi') LIMIT 10`,
     output: {
-      fields: [{ type: 'Field', field: 'Name' }, { type: 'Field', field: 'StreetAddress__c' }],
+      fields: [
+        { type: 'Field', field: 'Name' },
+        { type: 'Field', field: 'StreetAddress__c' },
+      ],
       sObject: 'Warehouse__c',
       where: {
         left: {
@@ -1635,6 +1682,25 @@ export const testCases: TestCase[] = [
       sObject: 'CONTACT',
     },
   },
+  {
+    testCase: 91,
+    soql: 'SELECT BillingState, BillingStreet, COUNT(Id) FROM Account GROUP BY BillingState, BillingStreet',
+    output: {
+      fields: [
+        { type: 'Field', field: 'BillingState' },
+        { type: 'Field', field: 'BillingStreet' },
+        {
+          type: 'FieldFunctionExpression',
+          functionName: 'COUNT',
+          rawValue: 'COUNT(Id)',
+          isAggregateFn: true,
+          parameters: ['Id'],
+        },
+      ],
+      sObject: 'Account',
+      groupBy: { field: ['BillingState', 'BillingStreet'] },
+    },
+  },
 ];
 
 export default testCases;
diff --git a/test/test.spec.ts b/test/test.spec.ts
index 8d05636..7dfb8dd 100644
--- a/test/test.spec.ts
+++ b/test/test.spec.ts
@@ -11,7 +11,7 @@ const replacements = [{ matching: / last /i, replace: ' LAST ' }];
 // Uncomment these to easily test one specific query - useful for troubleshooting/bugfixing
 
 // describe.only('parse queries', () => {
-//   const testCase = testCases.find(tc => tc.testCase === 90);
+//   const testCase = testCases.find(tc => tc.testCase === 91);
 //   it(`should correctly parse test case ${testCase.testCase} - ${testCase.soql}`, () => {
 //     const soqlQuery = parseQuery(testCase.soql, testCase.options);
 //     const soqlQueryWithoutUndefinedProps = JSON.parse(JSON.stringify(soqlQuery));