Skip to content

fix: include missing key columns for UPSERT from SELECT queries #1454

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.

Sign up for GitHub

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

Open
BobdenOs wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: include missing key columns for UPSERT from SELECT queries #1454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d427ac4
include missing key columns for UPSERT from SELECT
BobdenOs Dec 22, 2025
696b73c
Add missing default key for UPSERT from SELECT queries
BobdenOs Dec 23, 2025
21ffb9b
more complex test and their fixes
BobdenOs Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions db-service/lib/cqn2sql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,10 +1100,21 @@ class CQN2SQLRenderer {
const extractions = this._managed
if (this.values) this.values = [] // Clear previously computed values
const src = this.cqn4sql(UPSERT.from || UPSERT.as)
const missingKeys = this._managed.slice(src.SELECT.columns.length)
.filter(c => keys.includes(c.name))
.map(c => ({
__proto__: this.managed_session_context(elements[c.name]['@cds.on.insert']?.['='])
|| this.managed_session_context(elements[c.name].default?.ref?.[0])
|| (elements[c.name].default && { __proto__: elements[c.name].default, param: false })
|| { val: null, param: false },
as: c.name,
}))
const aliasedQuery = cds.ql.SELECT
.columns(src.SELECT.columns
.map((c, i) => ({ ref: [this.column_name(c)], as: this.columns[i] }))
)
.columns([
...src.SELECT.columns
.map((c, i) => ({ ref: [this.column_name(c)], as: this.columns[i] })),
...missingKeys,
])
.from(src)
sql = `SELECT ${extractions.map(c => `${c.upsert}`)} FROM (${this.SELECT(aliasedQuery)}) AS NEW LEFT JOIN ${this.quote(entity)} AS OLD ON ${keyCompare}`
if (extractions.length > columns.length) columns = this.columns = extractions.map(c => c.name)
Expand Down Expand Up @@ -1437,6 +1448,9 @@ class CQN2SQLRenderer {
const keyZero = keys[0] && this.quote(keys[0])

return [...columns, ...requiredColumns].map(({ name, sql }) => {
const isNull = sql === 'NULL'
const json = !sql

const element = elements?.[name] || {}

const converter = a => element[_convertInput]?.(a, element) || a
Expand All @@ -1458,8 +1472,8 @@ class CQN2SQLRenderer {

const qname = this.quote(name)

const insert = onInsert ? this.managed_default(name, converter(onInsert), sql) : sql
const update = onUpdate ? this.managed_default(name, converter(onUpdate), sql) : sql
const insert = onInsert ? json ? this.managed_default(name, converter(onInsert), sql) : !isNull ? sql : onInsert : sql
const update = onUpdate ? json ? this.managed_default(name, converter(onUpdate), sql) : !isNull ? sql : onUpdate : sql
const upsert = keyZero && (
// upsert requires the keys to be provided for the existance join (default values optional)
element.key
Expand All @@ -1471,7 +1485,7 @@ class CQN2SQLRenderer {
insert
} ELSE ${
// Else execute managed update or keep old if no new data if provided
onUpdate ? update : this.managed_default(name, `OLD.${qname}`, update)
onUpdate || !json ? update : this.managed_default(name, `OLD.${qname}`, update)
} END as ${qname}`
)

Expand Down
19 changes: 15 additions & 4 deletions hana/lib/HANAService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ class HANAService extends SQLService {
// temporal data
keys.push(...ObjectKeys(q._target.elements).filter(e => q._target.elements[e]['@cds.valid.from']))

const managed = this.managed(
const managed = this._managed || this.managed(
this.columns.map(c => ({ name: c })),
elements
)
Expand All @@ -871,10 +871,21 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction}) ERROR ON E
} else {
const src = this.cqn4sql(UPSERT.from || UPSERT.as)
if (this.values) this.values = []
const missingKeys = this._managed.slice(src.SELECT.columns.length)
.filter(c => keys.includes(c.name))
.map(c => ({
__proto__: this.managed_session_context(elements[c.name]['@cds.on.insert']?.['='])
|| this.managed_session_context(elements[c.name].default?.ref?.[0])
|| (elements[c.name].default && { __proto__: elements[c.name].default, param: false })
|| { val: null, param: false },
as: c.name,
}))
const aliasedQuery = cds.ql.SELECT
.columns(src.SELECT.columns
.map((c, i) => ({ ref: [this.column_name(c)], as: this.columns[i] }))
)
.columns([
...src.SELECT.columns
.map((c, i) => ({ ref: [this.column_name(c)], as: this.columns[i] })),
...missingKeys,
])
.from(src)
sql = `SELECT ${mixing} FROM (${this.SELECT(aliasedQuery)}) AS NEW LEFT JOIN ${this.quote(entity)} AS OLD ON ${keyCompare}`
this.entries = [this.values]
Expand Down
6 changes: 2 additions & 4 deletions hana/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@
"CHANGELOG.md"
],
"scripts": {
"test": "(([ -z \"${HANA_HOST}\" ] && npm start) || true) && npm run test:plain && npm run test:bookshop:quoted",
"test:bookshop:quoted": "cds_sql_names=quoted cds-test bookshop",
"test:plain": "cds-test",
"test:remote": "cds-test",
"test": "sh -c '(([ -z \"${HANA_HOST}\" ] && npm start) || true) && (([ -z $1 ] && npm run test:ci) || ([ -n $1 ] && cds-test $@))' --",
"test:ci": "cds-test && cds_sql_names=quoted cds-test bookshop",
"start": "npm run start:hce || npm run start:hxe",
"start:hce": "cd ./tools/docker/hce/ && ./start.sh",
"start:hxe": "cd ./tools/docker/hxe/ && ./start.sh"
Expand Down
43 changes: 43 additions & 0 deletions test/compliance/UPSERT.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('UPSERT', () => {
{ id: 1 },
{ id: 1, default: 'overwritten' },
]).into(keys)

await UPSERT.into(cuid)
.columns(['ID'])
.from(cds.ql`SELECT id || '-' || default as ![something that is not "ID"] FROM ${keys} WHERE id = ${1}`)
Expand All @@ -79,6 +80,7 @@ describe('UPSERT', () => {
{ id: 1 },
{ id: 1, default: 'overwritten' },
]).into(keys)

await UPSERT.into(cuid)
.columns(['ID'])
.from(cds.ql`SELECT id || '-' || default as ![something that is not "ID"] FROM ${keys} WHERE id = ${1}`)
Expand All @@ -88,6 +90,47 @@ describe('UPSERT', () => {
{ ID: '1-overwritten' },
])
})

test('default key', async () => {
const { keys } = cds.entities('basic.projection')
// fill other table first
await UPSERT([
{ id: 1 },
{ id: 1, default: 'overwritten' },
]).into(keys)
let select

await UPSERT.into(keys)
.columns(['id', 'default'])
.from(cds.ql`SELECT 2, default FROM ${keys} WHERE id = ${1}`)
select = await SELECT.from(keys).where`id = ${2}`.orderBy('default')
expect(select).deep.eq([
{ id: 2, default: 'defaulted', data: null },
{ id: 2, default: 'overwritten', data: null },
])

await UPSERT.into(keys)
.columns(['id'])
// FYI: default check is required to ensure only a single row is produced for HANA
.from(cds.ql`SELECT 3 FROM ${keys} WHERE id = ${1} and default = ${'overwritten'}`)
select = await SELECT.from(keys).where`id = ${3}`.orderBy('default')
expect(select).deep.eq([
{ id: 3, default: 'defaulted', data: null },
])

await UPSERT.into(keys)
.columns(['id', 'data'])
.from(cds.ql`SELECT 3, count() FROM ${keys} WHERE id = ${1}`)
// FYI: only HANA does unique key constraints on the source SELECT result
// example: the following query will return 2 rows with their row count
// the result for this UPSERT would be undefined behavior (data = 1 or 2)
// therefor HANA ensures that all keys of the source SELECT are unique
// .from(cds.ql`SELECT 3, row_number() over () FROM ${keys} WHERE id = ${1}`)
select = await SELECT.from(keys).where`id = ${3}`.orderBy('default')
expect(select).deep.eq([
{ id: 3, default: 'defaulted', data: '2' },
])
})
})

test('affected row', async () => {
Expand Down