Skip to content

chore: re-write filter at from.at(-1) into queries where #1373

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

Merged
merged 16 commits into from
Oct 24, 2025
Merged

chore: re-write filter at from.at(-1) into queries where #1373

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d5849f2
chore: re-write filter at `from.at(-1)` into queries `where`
patricebender Oct 6, 2025
b10362e
comment
patricebender Oct 6, 2025
851da26
clean up the ref after re-write
patricebender Oct 6, 2025
1860ba9
Merge remote-tracking branch 'origin/main' into patrice/infix-leaf
patricebender Oct 6, 2025
9a68024
cleanup ref properly
patricebender Oct 6, 2025
b3ae93b
try something
patricebender Oct 7, 2025
2db4549
also do this for DELETE
patricebender Oct 7, 2025
8bb2263
also update
patricebender Oct 7, 2025
3532c05
Merge branch 'main' into patrice/infix-leaf
patricebender Oct 23, 2025
8d184aa
Merge branch 'main' into patrice/infix-leaf
patricebender Oct 24, 2025
6f9d2a2
Merge remote-tracking branch 'origin/main' into patrice/infix-leaf
patricebender Oct 24, 2025
938fc77
simplify coding, improve error message
patricebender Oct 24, 2025
17bc6cd
consider all steps
patricebender Oct 24, 2025
bf9f5e3
indent
patricebender Oct 24, 2025
e62537f
consider case where there is no where
patricebender Oct 24, 2025
0280995
add a test for the weird query
patricebender Oct 24, 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
18 changes: 15 additions & 3 deletions db-service/lib/cqn4sql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ function cqn4sql(originalQuery, model) {
if (inferred.SELECT?.from.ref?.at(-1).id) {
assignQueryModifiers(inferred.SELECT, inferred.SELECT.from.ref.at(-1))
}
if (inferred.DELETE?.from.ref?.at(-1).id) {
assignQueryModifiers(inferred.DELETE, inferred.DELETE.from.ref.at(-1))
}
if (inferred.UPDATE?.entity.ref?.at(-1).id) {
assignQueryModifiers(inferred.UPDATE, inferred.UPDATE.entity.ref.at(-1))
}
inferred = infer(inferred, model)
const { getLocalizedName, isLocalized, getDefinition } = getModelUtils(model, originalQuery) // TODO: pass model to getModelUtils
// if the query has custom joins we don't want to transform it
Expand Down Expand Up @@ -1785,9 +1791,9 @@ function cqn4sql(originalQuery, model) {
}
}

// only append infix filter to outer where if it is the leaf of the from ref
if (refReverse[0].where)
filterConditions.push(getTransformedTokenStream(refReverse[0].where,{ $baseLink: $refLinksReverse[0] }))
// OData shortcut
if (refReverse[0].where?.length === 1 && refReverse[0].where[0].val)
filterConditions.push(getTransformedTokenStream(refReverse[0].where, { $baseLink: $refLinksReverse[0] }))

if (existingWhere.length > 0) filterConditions.push(existingWhere)
if (whereExistsSubSelects.length > 0) {
Expand Down Expand Up @@ -2418,6 +2424,12 @@ function assignQueryModifiers(SELECT, modifiers) {
} else if (key === 'having') {
if (!SELECT.having) SELECT.having = val
else SELECT.having.push('and', ...val)
} else if (key === 'where') {
// ignore OData shortcut variant: `… bookshop.Orders:items[2]`
if(val.length === 1 && val[0].val) continue
if (!SELECT.where) SELECT.where = val
// infix filter comes first in resulting where
else SELECT.where = [...(hasLogicalOr(val) ? [asXpr(val)] : val), 'and', ...(hasLogicalOr(SELECT.where) ? [asXpr(SELECT.where)] : SELECT.where)]
}
}
}
Expand Down
12 changes: 4 additions & 8 deletions db-service/test/cqn4sql/basic.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,24 +71,20 @@ describe('query clauses', () => {
Bar.ID as ID: cds.String
}`)
})
//(SMW) TODO I'd prefer to have the cond from the filter before the cond coming from the WHERE
// which, by the way, is the case in tests below where we have a path in FROM -> ???
it('handles infix filter at entity and WHERE clause', () => {
let query = cqn4sql(cds.ql`SELECT from bookshop.Books[price < 12.13] as Books {Books.ID} where stock < 11`, model)
let query = cqn4sql(cds.ql`SELECT from bookshop.Books[price < 12.13 or true] as Books {Books.ID} where stock < 11`, model)
expect(query).to.deep.equal(
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11) and (Books.price < 12.13)`,
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.price < 12.13 or true) and Books.stock < 11`,
)
})

//(SMW) TODO I'd prefer to have the cond from the filter before the cond coming from the WHERE
// which, by the way, is the case in tests below where we have a path in FROM -> ???
Comment on lines -83 to -84
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done according to your wish @stewsk

it('gets precedence right for infix filter at entity and WHERE clause', () => {
let query = cqn4sql(
cds.ql`SELECT from bookshop.Books[price < 12.13 or stock > 77] as Books {Books.ID} where stock < 11 or price > 17.89`,
model,
)
expect(query).to.deep.equal(
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11 or Books.price > 17.89) and (Books.price < 12.13 or Books.stock > 77)`,
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.price < 12.13 or Books.stock > 77) and (Books.stock < 11 or Books.price > 17.89)`,
)
//expect (query) .to.deep.equal (cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.price < 12.13 or Books.stock > 77) and (Books.stock < 11 or Books.price > 17.89)`) // (SMW) want this
})
Expand All @@ -101,7 +97,7 @@ describe('query clauses', () => {
model,
)
expect(query).to.deep.equal(
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE (Books.stock < 11) and (not (Books.price < 12.13))`,
cds.ql`SELECT from bookshop.Books as Books {Books.ID} WHERE not (Books.price < 12.13) and Books.stock < 11`,
)
})

Expand Down
4 changes: 3 additions & 1 deletion db-service/test/cqn4sql/exists.test/negative.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ describe('(exist predicate) negative tests', () => {
})

describe('restrictions', () => {
it('rejects the path expression at the leaf of scoped queries', () => {
// semantically equivalent to adding a where clause..
// IMO artificially rejecting this is not necessary, we can solve this uniformly also for regular where clause
it.skip('rejects the path expression at the leaf of scoped queries', () => {
// original idea was to just add the `genre.name` as WHERE clause to the query
// however, with left outer joins we might get too many results
//
Expand Down
32 changes: 16 additions & 16 deletions db-service/test/cqn4sql/exists.test/scoped-queries.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ describe('(exist predicate) scoped queries', () => {
WHERE EXISTS (
SELECT 1 from bookshop.Books as $B
where $B.author_ID = author.ID and ($B.ID=201 or $B.ID=202)
) and (author.ID=4711 or author.ID=4712) and (author.name='foo' or author.name='bar')`
) and ((author.ID=4711 or author.ID=4712) and (author.name='foo' or author.name='bar'))`

expectCqn(transformed).to.equal(expected)
})
Expand Down Expand Up @@ -450,11 +450,11 @@ describe('(exist predicate) scoped queries', () => {
$a.ID
}
WHERE EXISTS (
SELECT 1 from bookshop.Books as $B
where $B.author_ID = $a.ID
SELECT 1 from bookshop.Books as $B2
where $B2.author_ID = $a.ID
) and EXISTS (
SELECT 1 from bookshop.Books as $b2
where $b2.author_ID = $a.ID
SELECT 1 from bookshop.Books as $b
where $b.author_ID = $a.ID
)`

expectCqn(transformed).to.equal(expected)
Expand Down Expand Up @@ -497,15 +497,15 @@ describe('(exist predicate) scoped queries', () => {
$a.ID
}
WHERE EXISTS (
SELECT 1 from bookshop.Books as $B
where $B.author_ID = $a.ID
SELECT 1 from bookshop.Books as $B2
where $B2.author_ID = $a.ID
and EXISTS (
SELECT 1 from bookshop.Genres as $g
where $g.ID = $B.genre_ID
where $g.ID = $B2.genre_ID
)
) and EXISTS (
SELECT 1 from bookshop.Books as $b2
where $b2.author_ID = $a.ID
SELECT 1 from bookshop.Books as $b
where $b.author_ID = $a.ID
)`

expectCqn(transformed).to.equal(expected)
Expand Down Expand Up @@ -533,11 +533,11 @@ describe('(exist predicate) scoped queries', () => {
and EXISTS (
SELECT 1 from bookshop.Books as $B3 where $B3.author_ID = $a.ID
and EXISTS (
SELECT 1 from bookshop.Genres as $g where $g.ID = $B3.genre_ID
SELECT 1 from bookshop.Genres as $g2 where $g2.ID = $B3.genre_ID
)
)
) and EXISTS (
SELECT 1 from bookshop.Genres as $g2 where $g2.ID = $b.genre_ID
SELECT 1 from bookshop.Genres as $g where $g.ID = $b.genre_ID
)`

expectCqn(transformed).to.equal(expected)
Expand All @@ -556,14 +556,14 @@ describe('(exist predicate) scoped queries', () => {
$a.ID
}
WHERE EXISTS (
SELECT 1 from bookshop.Books as $B where $B.author_ID = $a.ID
SELECT 1 from bookshop.Books as $B2 where $B2.author_ID = $a.ID
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alias order changes because infix filter at leaf are put at the very front of the queries where

) and EXISTS (
SELECT 1 from bookshop.Books as $b2 where $b2.author_ID = $a.ID
SELECT 1 from bookshop.Books as $b where $b.author_ID = $a.ID
and
(
EXISTS (
SELECT 1 from bookshop.Authors as $c where $c.ID = $b2.coAuthor_ID_unmanaged
) or $b2.title = 'Sturmhöhe'
SELECT 1 from bookshop.Authors as $c where $c.ID = $b.coAuthor_ID_unmanaged
) or $b.title = 'Sturmhöhe'
)
)`

Expand Down
2 changes: 1 addition & 1 deletion db-service/test/cqn4sql/path-in-from.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('infix filter on entities', () => {
] as Books {ID} where price > 5 group by price having price order by price limit 5`, model)
expect(query).to.deep.equal(
cds.ql`SELECT from bookshop.Books as Books {Books.ID}
WHERE (Books.price > 5) and (Books.price < 12.13)
WHERE Books.price < 12.13 and Books.price > 5
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only wrap in parens if really needed

GROUP BY Books.price, Books.title
HAVING Books.price and Books.title
ORDER BY Books.price, Books.title DESC
Expand Down
20 changes: 8 additions & 12 deletions db-service/test/cqn4sql/search.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,17 @@ describe('Replace attribute search by search predicate', () => {
from: { as: 'Genres', ref: ['bookshop.Genres'] },
where: [
{
xpr: [
{
args: [
{
list: [{ ref: ['Genres', 'name'] }, { ref: ['Genres', 'descr'] }, { ref: ['Genres', 'code'] }],
},
{ xpr: [{ val: 'x' }, 'or', { val: 'y' }] },
],
func: 'search',
},
],
xpr: [{ ref: ['Genres', 'ID'] }, '<', { val: 4 }, 'or', { ref: ['Genres', 'ID'] }, '>', { val: 5 }],
},
'and',
{
xpr: [{ ref: ['Genres', 'ID'] }, '<', { val: 4 }, 'or', { ref: ['Genres', 'ID'] }, '>', { val: 5 }],
args: [
{
list: [{ ref: ['Genres', 'name'] }, { ref: ['Genres', 'descr'] }, { ref: ['Genres', 'code'] }],
},
{ xpr: [{ val: 'x' }, 'or', { val: 'y' }] },
],
func: 'search',
},
],
},
Expand Down
Loading