Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Mar 8, 2024
17 changes: 8 additions & 9 deletions db-service/lib/cqn2sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(?)`)
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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(?)`)
}

Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions db-service/test/cqn2sql/__snapshots__/upsert.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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"""",
}
`;
7 changes: 7 additions & 0 deletions db-service/test/cqn2sql/testModel.cds
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ entity Foo2 {
virtual something : String(11);
}

entity !["Foo2Quoted"] {
key !["ID"]: Integer;
!["name"]: String;
!["a"]: Integer;
BobdenOs marked this conversation as resolved.
Show resolved Hide resolved
virtual !["something"] : String(11);
}

entity FooCollate {
key ID: UUID;
collateString: String;
Expand Down
13 changes: 13 additions & 0 deletions db-service/test/cqn2sql/upsert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
9 changes: 9 additions & 0 deletions test/compliance/keywords.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
})
})
1 change: 1 addition & 0 deletions test/compliance/resources/db/keywords/index.cds
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ entity Alter {
entity ASC {
key ID : Integer;
alias: Integer;
![select]: Integer;
}
Loading