From 8fe1bc5e459be16f03428ade161ca3690ce92356 Mon Sep 17 00:00:00 2001
From: Patrice Bender <patrice.bender@sap.com>
Date: Wed, 8 Jan 2025 13:00:03 +0100
Subject: [PATCH] fix: reject virtual elements in all expressions

---
 db-service/lib/cqn4sql.js                     |  9 ++-
 db-service/test/cqn4sql/not-persisted.test.js | 61 ++++++-------------
 2 files changed, 24 insertions(+), 46 deletions(-)

diff --git a/db-service/lib/cqn4sql.js b/db-service/lib/cqn4sql.js
index 167fd884e..70a06737b 100644
--- a/db-service/lib/cqn4sql.js
+++ b/db-service/lib/cqn4sql.js
@@ -1450,7 +1450,7 @@ function cqn4sql(originalQuery, model) {
         transformedTokenStream.push({ ...token })
       } else {
         // expand `struct = null | struct2`
-        const { definition } = token.$refLinks?.[token.$refLinks.length - 1] || {}
+        const definition = token.$refLinks?.at(-1).definition
         const next = tokenStream[i + 1]
         if (allOps.some(([firstOp]) => firstOp === next) && (definition?.elements || definition?.keys)) {
           const ops = [next]
@@ -1477,6 +1477,9 @@ function cqn4sql(originalQuery, model) {
         } else {
           // reject associations in expression, except if we are in an infix filter -> $baseLink is set
           assertNoStructInXpr(token, $baseLink)
+          // reject virtual elements in expressions as they will lead to a sql error down the line
+          if(definition?.virtual)
+            throw new Error(`Virtual elements are not allowed in expressions`)
 
           let result = is_regexp(token?.val) ? token : copy(token) // REVISIT: too expensive! //
           if (token.ref) {
@@ -1620,10 +1623,10 @@ function cqn4sql(originalQuery, model) {
       rejectStructInExpression()
 
     function rejectAssocInExpression() {
-      throw new Error("An association can't be used as a value in an expression")
+      throw new Error(`An association can't be used as a value in an expression`)
     }
     function rejectStructInExpression() {
-      throw new Error("A structured element can't be used as a value in an expression")
+      throw new Error(`A structured element can't be used as a value in an expression`)
     }
   }
 
diff --git a/db-service/test/cqn4sql/not-persisted.test.js b/db-service/test/cqn4sql/not-persisted.test.js
index 870709eb8..cc9eff72a 100644
--- a/db-service/test/cqn4sql/not-persisted.test.js
+++ b/db-service/test/cqn4sql/not-persisted.test.js
@@ -86,20 +86,6 @@ describe('not persisted', () => {
         order by ID`)
     })
 
-    it('dont remove in xpr in ORDER BY', () => {
-      let query = cqn4sql(
-        CQL`SELECT from bookshop.Foo { ID, virtualField as x }
-        order by ID, x, (Foo.toFoo.virtualField * 42)`,
-        model,
-      )
-      expect(query).to.deep.equal(
-        CQL`SELECT from bookshop.Foo as Foo left join bookshop.Foo as toFoo on toFoo.ID = Foo.toFoo_ID
-        {
-          Foo.ID
-        }
-        order by ID, (toFoo.virtualField * 42)`,
-      )
-    })
 
     it('Navigation to virtual field does not cause join', () => {
       let query = cqn4sql(
@@ -117,41 +103,30 @@ describe('not persisted', () => {
     // Virtual fields in expressions are left untouched and will cause an error in the DB.
     // The idea to replace conditions involving virtual fields by "1=1" doesn't work, as we
     // are not able to detect where the conditions start/end (-> we don't understand xpr)
-    it('leave untouched in expressions', () => {
-      let query = cqn4sql(
-        CQL`SELECT from bookshop.Foo {
-        ID,
-        virtualField - 2 * stru.v + stru.nested.nv as c
-      } where virtualField = 2 * stru.v + stru.nested.nv and virtualField`,
-        model,
-      )
-      expect(query).to.deep.equal(CQL`SELECT from bookshop.Foo as Foo {
-        Foo.ID,
-        Foo.virtualField - 2 * Foo.stru_v + Foo.stru_nested_nv as c
-      } where Foo.virtualField = 2 * Foo.stru_v + Foo.stru_nested_nv and Foo.virtualField`)
+    it('reject virtual elements in expressions', () => {
+      let query = CQL`SELECT from bookshop.Foo {
+        ID
+      } where virtualField = 2 * stru.v + stru.nested.nv and virtualField`;
+      expect(() => cqn4sql(query, model)).to.throw('Virtual elements are not allowed in expressions')
     })
 
-    it('Navigation to virtual field does cause join in expression', () => {
-      let query = cqn4sql(
-        CQL`SELECT from bookshop.Foo {
+    it('reject virtual elements in column expressions', () => {
+      let query = CQL`SELECT from bookshop.Foo {
         ID,
         toFoo.virtualField + 42 / 20 as virtualField,
-      }`,
-        model,
-      )
-      expect(query).to.deep.equal(CQL`SELECT from bookshop.Foo as Foo
-        left join bookshop.Foo as toFoo on toFoo.ID = Foo.toFoo_ID
-      {
-        Foo.ID,
-        toFoo.virtualField + 42 / 20 as virtualField
-      }`)
+      }`
+      expect(() => cqn4sql(query, model)).to.throw('Virtual elements are not allowed in expressions')
     })
 
-    it('leave untouched also in simple conditions', () => {
-      let query = cqn4sql(CQL`SELECT from bookshop.Foo { ID } where ID = 5 and virtualField = 6`, model)
-      expect(query).to.deep.equal(
-        CQL`SELECT from bookshop.Foo as Foo { Foo.ID } where Foo.ID = 5 and Foo.virtualField = 6`,
-      )
+    it('reject virtual elements in simple conditions', () => {
+      let query = CQL`SELECT from bookshop.Foo { ID } where ID = 5 and virtualField = 6`
+      expect(() => cqn4sql(query, model)).to.throw('Virtual elements are not allowed in expressions')
+    })
+
+    it('reject virtual elements in order by', () => {
+      let query =  CQL`SELECT from bookshop.Foo { ID, virtualField as x }
+        order by ID, x, (Foo.toFoo.virtualField * 42)`
+      expect(() => cqn4sql(query, model)).to.throw('Virtual elements are not allowed in expressions')
     })
   })