Skip to content

Commit

Permalink
fix(UPDATE): only perform subselect matching if necessary (#989)
Browse files Browse the repository at this point in the history
the issue was that while calculating the `UPDATE.elements`, we
accidentially assumed that the calculated element would lead to a join,
hence we did our subselect matching. This logic is not necessary for non
`SELECT` queries --> there we need joins if we do a `SELECT * from
<entity with calculated element /w path expression>`

---

fix(`UPDATE`): no assocs in list which matches subquery results
  • Loading branch information
patricebender authored Jan 23, 2025
1 parent fe4f66b commit 4bcb88a
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 2 deletions.
2 changes: 1 addition & 1 deletion db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function cqn4sql(originalQuery, model) {
const primaryKey = { list: [] }
for (const k of Object.keys(queryTarget.elements)) {
const e = queryTarget.elements[k]
if (e.key === true && !e.virtual) {
if (e.key === true && !e.virtual && e.isAssociation !== true) {
subquery.SELECT.columns.push({ ref: [e.name] })
primaryKey.list.push({ ref: [transformedFrom.as, e.name] })
}
Expand Down
3 changes: 2 additions & 1 deletion db-service/lib/infer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,8 @@ function infer(originalQuery, model) {
if (element.type !== 'cds.LargeBinary') {
queryElements[k] = element
}
if (isCalculatedOnRead(element)) {
// only relevant if we actually select the calculated element
if (originalQuery.SELECT && isCalculatedOnRead(element)) {
linkCalculatedElement(element)
}
}
Expand Down
37 changes: 37 additions & 0 deletions db-service/test/cqn4sql/UPDATE.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,41 @@ describe('UPDATE with path expression', () => {
let res = cqn4sql(u, model)
expect(JSON.parse(JSON.stringify(res))).to.deep.equal(JSON.parse(JSON.stringify(expected)))
})

it('path expression via calculated element leads to subquery if used in where', () => {
const q = UPDATE('bookshop.Orders.Items').set({ price: 5 }).where('price = 4.99')

const res = cqn4sql(q, model)

const expected = UPDATE.entity({ ref: ['bookshop.Orders.Items'] }).alias('Items2')
expected.UPDATE.where = [
{
list: [{ ref: ['Items2', 'up__ID'] }, { ref: ['Items2', 'book_ID'] }],
},
'in',
cds.ql`
(SELECT
Items.up__ID,
Items.book_ID
FROM bookshop.Orders.Items AS Items
LEFT JOIN bookshop.Books AS book ON book.ID = Items.book_ID
WHERE (book.stock * 2) = 4.99
)
`,
]

expect(JSON.parse(JSON.stringify(res))).to.deep.equal(JSON.parse(JSON.stringify(expected)))
})

it('if there is no path expression in the where, we dont need subselect magic', () => {
const q = UPDATE('bookshop.Orders.Items').set({ quantity: 3 }).where('1 = 1')
const res = cqn4sql(q, model)
expect(JSON.parse(JSON.stringify(res))).to.eql({
UPDATE: {
entity: { ref: ['bookshop.Orders.Items'], as: 'Items' },
where: [{ val: 1 }, '=', { val: 1 }],
},
})
expect(res.UPDATE).to.have.property('data')
})
})
8 changes: 8 additions & 0 deletions db-service/test/cqn4sql/model/update.cds
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ entity Authors {
alive : Boolean;
}

entity Orders {
key ID: UUID;
Items: composition of many {
key book: Association to Books;
price: Decimal = book.stock * 2;
}
}

service CatalogService {
@odata.draft.enabled
entity Books as projection on bookshop.Books;
Expand Down

0 comments on commit 4bcb88a

Please sign in to comment.