fix: deep groupby (#919)

Fixes groupby requests with multiple path expressions in combination
with $filter and orderby. Proper source checking and separate path alias
from reference alias.

---------

Co-authored-by: Bob den Os <bob.den.os@sap.com>
Co-authored-by: Johannes Vogel <31311694+johannes-vogel@users.noreply.github.com>

hana/lib/HANAService.js

@@ -333,17 +333,17 @@ class HANAService extends SQLService {
       }
 
       let { limit, one, orderBy, expand, columns = ['*'], localized, count, parent } = q.SELECT
-
 
       // When one of these is defined wrap the query in a sub query
       if (expand || (parent && (limit || one || orderBy))) {
         const walkAlias = q => {
           if (q.args) return q.as || walkAlias(q.args[0])
           if (q.SELECT?.from) return walkAlias(q.SELECT?.from)
-          return q.as
+          return q.as || cds.error`Missing alias for subquery`
         }
-        q.as = walkAlias(q)
-        q.alias = `${parent ? parent.alias + '.' : ''}${q.as}`
+        const alias = q.as // Use query alias as path name
+        q.as = walkAlias(q) // Use from alias for query re use alias
+        q.alias = `${parent ? parent.alias + '.' : ''}${alias || q.as}`
         const src = q
 
         const { element, elements } = q
@@ -510,24 +510,41 @@ class HANAService extends SQLService {
                     // if (col.ref?.length === 1) { col.ref.unshift(parent.as) }
                     if (col.ref?.length > 1) {
                       const colName = this.column_name(col)
-                      if (col.ref[0] !== parent.as) {
+                      if (!parent.SELECT.columns.some(c => this.column_name(c) === colName)) {
+                        const isSource = from => {
+                          if (from.as === col.ref[0]) return true
+                          return from.args?.some(a => {
+                            if (a.args) return isSource(a)
+                            return a.as === col.ref[0]
+                          })
+                        }
+
                         // Inject foreign columns into parent selects (recursively)
                         const as = `$$${col.ref.join('.')}$$`
+                        let rename = col.ref[0] !== parent.as
                         let curPar = parent
                         while (curPar) {
-                          if (curPar.SELECT.from?.args?.some(a => a.as === col.ref[0])) {
-                            if (!curPar.SELECT.columns.find(c => c.as === as)) curPar.SELECT.columns.push({ __proto__: col, ref: col.ref, as })
+                          if (isSource(curPar.SELECT.from)) {
+                            if (curPar.SELECT.columns.find(c => c.as === as)) {
+                              rename = true
+                            } else {
+                              rename = rename || curPar === parent
+                              curPar.SELECT.columns.push(rename ? { __proto__: col, ref: col.ref, as } : { __proto__: col, ref: [...col.ref] })
+                            }
                             break
                           } else {
                             curPar.SELECT.columns.push({ __proto__: col, ref: [curPar.SELECT.parent.as, as], as })
                             curPar = curPar.SELECT.parent
                           }
                         }
-                        col.as = colName
-                        col.ref = [parent.as, as]
-                      } else if (!parent.SELECT.columns.some(c => this.column_name(c) === colName)) {
-                        // Inject local columns into parent select
-                        parent.SELECT.columns.push({ __proto__: col })
+                        if (rename) {
+                          col.as = colName
+                          col.ref = [parent.as, as]
+                        } else {
+                          col.ref = [parent.as, colName]
+                        }
+                      } else {
+                        col.ref[1] = colName
                       }
                     }
                   })

test/bookshop/srv/admin-service.cds

@@ -2,6 +2,7 @@ using { sap.capire.bookshop as my } from '../db/schema';
 service AdminService @(requires:'admin', path:'/admin') {
   entity Books as projection on my.Books;
   entity Authors as projection on my.Authors;
+  entity A as projection on my.A;
 
   @cds.redirection.target: false
   entity RenameKeys as projection on my.Books {

test/scenarios/bookshop/read.test.js

@@ -79,6 +79,21 @@ describe('Bookshop - Read', () => {
     expect(res.data.value.every(row => row.author.name)).to.be.true
   })
 
+  test('groupby with multiple path expressions', async () => {
+    const res = await GET('/admin/A?$apply=groupby((toB/toC/ID,toC/toB/ID))', admin)
+    expect(res.status).to.be.eq(200)
+  })
+
+  test('groupby with multiple path expressions and orderby', async () => {
+    const res = await GET('/admin/A?$apply=groupby((toB/toC/ID,toB/toC/ID))&$orderby=toB/toC/ID', admin)
+    expect(res.status).to.be.eq(200)
+  })
+
+  test('groupby with multiple path expressions and filter', async () => {
+    const res = await GET('/admin/A?$apply=groupby((toB/toC/ID,toB/toC/ID))&$filter=ID eq 1', admin)
+    expect(res.status).to.be.eq(200)
+  })
+
   test('Path expression', async () => {
     const q = CQL`SELECT title, author.name as author FROM sap.capire.bookshop.Books where author.name LIKE '%a%'`
     const res = await cds.run(q)