fix(cqn2sql): Smart quoting of columns inside UPSERT rows (#519)

Defer the column quoting until rendering them into the sql. To prevent
them from being quoted twice.

---------

Co-authored-by: Johannes Vogel <[email protected]>
Co-authored-by: Johannes Vogel <[email protected]>

db-service/lib/cqn2sql.js

@@ -404,12 +404,12 @@ class CQN2SQLRenderer {
       : ObjectKeys(INSERT.entries[0])
 
     /** @type {string[]} */
-    this.columns = columns.filter(elements ? c => !elements[c]?.['@cds.extension'] : () => true).map(c => this.quote(c))
+    this.columns = columns.filter(elements ? c => !elements[c]?.['@cds.extension'] : () => true)
 
     if (!elements) {
       this.entries = INSERT.entries.map(e => columns.map(c => e[c]))
       const param = this.param.bind(this, { ref: ['?'] })
-      return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns}) VALUES (${columns.map(param)})`)
+      return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns.map(c => this.quote(c))}) VALUES (${columns.map(param)})`)
     }
 
     const extractions = this.managed(
@@ -448,7 +448,7 @@ class CQN2SQLRenderer {
       this.entries = [[...this.values, stream]]
     }
 
-    return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns
+    return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns.map(c => this.quote(c))
       }) SELECT ${extraction} FROM json_each(?)`)
   }
 
@@ -575,12 +575,12 @@ class CQN2SQLRenderer {
       return converter?.(extract, element) || extract
     })
 
-    this.columns = columns.map(c => this.quote(c))
+    this.columns = columns
 
     if (!elements) {
       this.entries = INSERT.rows
       const param = this.param.bind(this, { ref: ['?'] })
-      return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns}) VALUES (${columns.map(param)})`)
+      return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns.map(c => this.quote(c))}) VALUES (${columns.map(param)})`)
     }
 
     if (INSERT.rows[0] instanceof Readable) {
@@ -592,7 +592,7 @@ class CQN2SQLRenderer {
       this.entries = [[...this.values, stream]]
     }
 
-    return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns
+    return (this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${this.columns.map(c => this.quote(c))
       }) SELECT ${extraction} FROM json_each(?)`)
   }
 
@@ -619,7 +619,7 @@ class CQN2SQLRenderer {
     const columns = (this.columns = (INSERT.columns || ObjectKeys(elements)).filter(
       c => c in elements && !elements[c].virtual && !elements[c].isAssociation,
     ))
-    this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${columns}) ${this.SELECT(
+    this.sql = `INSERT INTO ${this.quote(entity)}${alias ? ' as ' + this.quote(alias) : ''} (${columns.map(c => this.quote(c))}) ${this.SELECT(
       this.cqn4sql(INSERT.as),
     )}`
     this.entries = [this.values]
@@ -660,8 +660,7 @@ class CQN2SQLRenderer {
     if (!keys) return this.sql = sql
     keys = Object.keys(keys).filter(k => !keys[k].isAssociation && !keys[k].virtual)
 
-    let updateColumns = q.UPSERT.entries ? Object.keys(q.UPSERT.entries[0]) : this.columns
-    updateColumns = updateColumns.filter(c => {
+    const updateColumns = this.columns.filter(c => {
       if (keys.includes(c)) return false //> keys go into ON CONFLICT clause
       let e = elements[c]
       if (!e) return true //> pass through to native SQL columns not in CDS model

db-service/test/cqn2sql/__snapshots__/upsert.test.js.snap

@@ -21,3 +21,14 @@ exports[`upsert test with keys only 1`] = `
   "sql": "INSERT INTO Foo2 (ID) SELECT value->>'$[0]' FROM json_each(?) WHERE true ON CONFLICT(ID) DO NOTHING",
 }
 `;
+
+exports[`upsert test with rows (quoted) 1`] = `
+{
+  "entries": [
+    [
+      "[[1,null,2]]",
+    ],
+  ],
+  "sql": "INSERT INTO """Foo2Quoted""" ("""ID""","""name""","""a""") SELECT value->>'$[0]',value->>'$[1]',value->>'$[2]' FROM json_each(?) WHERE true ON CONFLICT("""ID""") DO UPDATE SET """name""" = excluded."""name""","""a""" = excluded."""a"""",
+}
+`;

db-service/test/cqn2sql/testModel.cds

@@ -13,6 +13,13 @@ entity Foo2 {
   virtual something : String(11);
 }
 
+entity !["Foo2Quoted"] {
+  key !["ID"]: Integer;
+  !["name"]: String;
+  !["a"]: Integer;
+  virtual !["something"] : String(11);
+}
+
 entity FooCollate {
   key ID: UUID;
   collateString: String;

db-service/test/cqn2sql/upsert.test.js

@@ -39,4 +39,17 @@ describe('upsert', () => {
     const { sql, entries } = cqn2sql(cqnUpsert)
     expect({ sql, entries: [[await text(entries[0][0])]] }).toMatchSnapshot()
   })
+
+  test('test with rows (quoted)', async () => {
+    const cqnUpsert = {
+      UPSERT: {
+        into: '"Foo2Quoted"',
+        columns: ['"ID"', '"name"', '"a"'],
+        rows: [[1, null, 2]],
+      },
+    }
+
+    const { sql, entries } = cqn2sql(cqnUpsert)
+    expect({ sql, entries: [[await text(entries[0][0])]] }).toMatchSnapshot()
+  })
 })

test/compliance/keywords.test.js

@@ -45,4 +45,13 @@ describe('keywords', () => {
     const select = await SELECT.from(Alter).where('number = 42')
     expect(select[0]).to.eql({ ID: 1, number: 42, order_ID: null })
   })
+
+  test('upsert', async () => {
+    const { ASC } = cds.entities
+    await UPSERT.into(ASC)
+      .columns(['ID', 'select'])
+      .rows([[42,4711]])
+    const select = await SELECT.one.from(ASC, ['ID', 'select']).where('ID = 42')
+    expect(select).to.eql({ ID: 42, select: 4711 })
+  })
 })

test/compliance/resources/db/keywords/index.cds

@@ -12,4 +12,5 @@ entity Alter {
 entity ASC {
   key ID : Integer;
   alias: Integer;
+  ![select]: Integer;
 }