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

feat: path expressions inside filter of scoped queries #991

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
11 changes: 8 additions & 3 deletions db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ function cqn4sql(originalQuery, model) {
transformedProp.where = getTransformedTokenStream(where)
}

const queryNeedsJoins = inferred.joinTree && !inferred.joinTree.isInitial
// Transform the from clause: association path steps turn into `WHERE EXISTS` subqueries.
// The already transformed `where` clause is then glued together with the resulting subqueries.
const { transformedWhere, transformedFrom } = getTransformedFrom(from || entity, transformedProp.where)
const queryNeedsJoins = inferred.joinTree && !inferred.joinTree.isInitial

if (inferred.SELECT) {
transformedQuery = transformSelectQuery(queryProp, transformedFrom, transformedWhere, transformedQuery)
Expand Down Expand Up @@ -1721,8 +1721,13 @@ 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, $refLinksReverse[0]))
if (refReverse[0].where) {
const whereIsTransformedLater = (inferred.joinTree && !inferred.joinTree.isInitial) && (originalQuery.DELETE || originalQuery.UPDATE)
if (whereIsTransformedLater)
filterConditions.push(refReverse[0].where)
else
filterConditions.push(getTransformedTokenStream(refReverse[0].where, $refLinksReverse[0]))
}

if (existingWhere.length > 0) filterConditions.push(existingWhere)
if (whereExistsSubSelects.length > 0) {
Expand Down
38 changes: 29 additions & 9 deletions db-service/lib/infer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,17 @@ function infer(originalQuery, model) {

let $combinedElements

// path expressions in from.ref.at(-1).where
// are collected here and merged once the joinTree is initialized
const mergeOnceJoinTreeIsInitialized = []

const sources = inferTarget(_.from || _.into || _.entity, {})
const joinTree = new JoinTree(sources)
const aliases = Object.keys(sources)
if (mergeOnceJoinTreeIsInitialized.length) {
mergeOnceJoinTreeIsInitialized.forEach(arg => joinTree.mergeColumn(arg, originalQuery.outerQueries))
}

Object.defineProperties(inferred, {
// REVISIT: public, or for local reuse, or in cqn4sql only?
sources: { value: sources, writable: true },
Expand Down Expand Up @@ -401,7 +409,7 @@ function infer(originalQuery, model) {
*/

function inferArg(arg, queryElements = null, $baseLink = null, context = {}) {
const { inExists, inExpr, inCalcElement, baseColumn, inInfixFilter, inQueryModifier, inFrom } = context
const { inExists, inExpr, inCalcElement, baseColumn, inInfixFilter, inQueryModifier, inFrom, atFromLeaf } = context
if (arg.param || arg.SELECT) return // parameter references are only resolved into values on execution e.g. :val, :1 or ?
if (arg.args) applyToFunctionArgs(arg.args, inferArg, [null, $baseLink, context])
if (arg.list) arg.list.forEach(arg => inferArg(arg, null, $baseLink, context))
Expand Down Expand Up @@ -448,7 +456,7 @@ function infer(originalQuery, model) {
if (inInfixFilter) {
const nextStep = arg.ref[1]?.id || arg.ref[1]
if (isNonForeignKeyNavigation(element, nextStep)) {
if (inExists) {
if (inExists || inFrom) {
Object.defineProperty($baseLink, 'pathExpressionInsideFilter', { value: true })
} else {
rejectNonFkNavigation(element, element.on ? $baseLink.definition.name : nextStep)
Expand Down Expand Up @@ -512,7 +520,7 @@ function infer(originalQuery, model) {
if ($baseLink && inInfixFilter) {
const nextStep = arg.ref[i + 1]?.id || arg.ref[i + 1]
if (isNonForeignKeyNavigation(element, nextStep)) {
if (inExists) {
if (inExists || inFrom) {
Object.defineProperty($baseLink, 'pathExpressionInsideFilter', { value: true })
} else {
rejectNonFkNavigation(element, element.on ? $baseLink.definition.name : nextStep)
Expand Down Expand Up @@ -567,13 +575,19 @@ function infer(originalQuery, model) {
inExpr: !!token.xpr,
inInfixFilter: true,
inFrom,
atFromLeaf: inFrom && !arg.ref[i + 1],
})
} else if (token.func) {
if (token.args) {
applyToFunctionArgs(token.args, inferArg, [
false,
arg.$refLinks[i],
{ inExists: skipJoinsForFilter || inExists, inExpr: true, inInfixFilter: true, inFrom },
arg.$refLinks[i], {
inExists: skipJoinsForFilter || inExists,
inExpr: true,
inInfixFilter: true,
inFrom,
atFromLeaf: inFrom && !arg.ref[i + 1],
},
])
}
}
Expand Down Expand Up @@ -629,7 +643,14 @@ function infer(originalQuery, model) {
})

// we need inner joins for the path expressions inside filter expressions after exists predicate
if ($baseLink?.pathExpressionInsideFilter) Object.defineProperty(arg, 'join', { value: 'inner' })
if ($baseLink?.pathExpressionInsideFilter) {
Object.defineProperty(arg, 'join', { value: 'inner' })
if (inFrom && atFromLeaf && !inExists) {
// join tree not yet initialized
Object.defineProperty(arg, 'isJoinRelevant', { value: true })
mergeOnceJoinTreeIsInitialized.push(arg)
}
}

// ignore whole expand if target of assoc along path has ”@cds.persistence.skip”
if (arg.expand) {
Expand Down Expand Up @@ -876,8 +897,7 @@ function infer(originalQuery, model) {
step[nestedProp].forEach(a => {
// reset sub path for each nested argument
// e.g. case when <path> then <otherPath> else <anotherPath> end
if(!a.ref)
subPath = { $refLinks: [...basePath.$refLinks], ref: [...basePath.ref] }
if (!a.ref) subPath = { $refLinks: [...basePath.$refLinks], ref: [...basePath.ref] }
mergePathsIntoJoinTree(a, subPath)
})
}
Expand All @@ -888,7 +908,7 @@ function infer(originalQuery, model) {
const calcElementIsJoinRelevant = isColumnJoinRelevant(p)
if (calcElementIsJoinRelevant) {
if (!calcElement.value.isJoinRelevant)
Object.defineProperty(step, 'isJoinRelevant', { value: true, writable: true, })
Object.defineProperty(step, 'isJoinRelevant', { value: true, writable: true })
joinTree.mergeColumn(p, originalQuery.outerQueries)
} else {
// we need to explicitly set the value to false in this case,
Expand Down
2 changes: 1 addition & 1 deletion db-service/test/cds-infer/negative.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ describe('negative', () => {
})

describe('infix filters', () => {
it('rejects non fk traversal in infix filter in from', () => {
it.skip('rejects non fk traversal in infix filter in from', () => {
expect(() => _inferred(CQL`SELECT from bookshop.Books[author.name = 'Kurt']`, model)).to.throw(
/Only foreign keys of author can be accessed in infix filter, but found name/,
)
Expand Down
59 changes: 59 additions & 0 deletions db-service/test/cqn4sql/DELETE.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,63 @@ describe('DELETE', () => {
}
expect(query.DELETE).to.deep.equal(expected.DELETE)
})

describe('with path expressions', () => {
let forNodeModel
beforeAll(() => {
// subqueries reference flat author_ID, which is not part of client csn
forNodeModel = cds.compile.for.nodejs(JSON.parse(JSON.stringify(cds.model)))
})

it('inner joins for the path expression at the leaf of scoped queries', () => {
let query = DELETE.from('bookshop.Authors:books[genre.name = null]')
const transformed = cqn4sql(query, forNodeModel)

const subquery = cds.ql`
SELECT books.ID from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors where Authors.ID = books.author_ID
) and genre.name = null`
const expected = DELETE.from('bookshop.Books').alias('books2')
expected.DELETE.where = [{ list: [{ ref: ['books2', 'ID'] }] }, 'in', subquery]

expect(transformed).to.deep.equal(expected)
})

it('inner joins for the path expression at the leaf of scoped queries, two assocs', () => {
let query = DELETE.from('bookshop.Authors:books[genre.parent.name = null]')
const transformed = cqn4sql(query, forNodeModel)

const subquery = cds.ql`
SELECT books.ID from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
inner join bookshop.Genres as parent on parent.ID = genre.parent_ID
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors where Authors.ID = books.author_ID
) and parent.name = null`
const expected = DELETE.from('bookshop.Books').alias('books2')
expected.DELETE.where = [{ list: [{ ref: ['books2', 'ID'] }] }, 'in', subquery]

expect(transformed).to.deep.equal(expected)
})
it('inner joins for the path expression NOT at the leaf of scoped queries, two assocs', () => {
let query = DELETE.from(`bookshop.Authors[books.title = 'bar']:books[genre.parent.name = null]`).alias('MyBook')

const transformed = cqn4sql(query, forNodeModel)
const subquery = cds.ql`
SELECT MyBook.ID from bookshop.Books as MyBook
inner join bookshop.Genres as genre on genre.ID = MyBook.genre_ID
inner join bookshop.Genres as parent on parent.ID = genre.parent_ID
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors
inner join bookshop.Books as books on books.author_ID = Authors.ID
where Authors.ID = MyBook.author_ID and books.title = 'bar'
) and parent.name = null`
const expected = DELETE.from('bookshop.Books').alias('MyBook2')
expected.DELETE.where = [{ list: [{ ref: ['MyBook2', 'ID'] }] }, 'in', subquery]

expect(transformed).to.deep.equal(expected)
})
})
})
61 changes: 56 additions & 5 deletions db-service/test/cqn4sql/UPDATE.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,14 @@ describe('UPDATE', () => {
})
})
describe('UPDATE with path expression', () => {
let model
let forNodeModel
beforeAll(async () => {
model = cds.model = await cds.load(__dirname + '/model/update').then(cds.linked)
model = cds.compile.for.nodejs(model)
cds.model = await cds.load(__dirname + '/../bookshop/srv/cat-service').then(cds.linked)
forNodeModel = cds.compile.for.nodejs(JSON.parse(JSON.stringify(cds.model)))
})

it('with path expressions with draft enabled entity', () => {
it('with path expressions with draft enabled entity', async () => {
const draftModel = await cds.load(__dirname + '/model/update').then(cds.linked)
const { UPDATE } = cds.ql
let u = UPDATE.entity({ ref: ['bookshop.CatalogService.Books'] }).where(`author.name LIKE '%Bron%'`)

Expand All @@ -197,7 +198,57 @@ describe('UPDATE with path expression', () => {
as: 'Books2',
ref: ['bookshop.CatalogService.Books'],
}
let res = cqn4sql(u, model)
let res = cqn4sql(u, draftModel)
expect(JSON.parse(JSON.stringify(res))).to.deep.equal(JSON.parse(JSON.stringify(expected)))
})

it('inner joins for the path expression at the leaf of scoped queries', () => {
let query = UPDATE.entity('bookshop.Authors:books[genre.name = null]')

const transformed = cqn4sql(query, forNodeModel)
const subquery = cds.ql`
SELECT books.ID from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors where Authors.ID = books.author_ID
) and genre.name = null`
const expected = UPDATE.entity('bookshop.Books').alias('books2')
expected.UPDATE.where = [{ list: [{ ref: ['books2', 'ID'] }] }, 'in', subquery]

expect(transformed).to.deep.equal(expected)
})
it('inner joins for the path expression at the leaf of scoped queries, two assocs (UPDATE)', () => {
let query = UPDATE.entity('bookshop.Authors:books[genre.parent.name = null]')

const transformed = cqn4sql(query, forNodeModel)
const subquery = cds.ql`
SELECT books.ID from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
inner join bookshop.Genres as parent on parent.ID = genre.parent_ID
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors where Authors.ID = books.author_ID
) and parent.name = null`
const expected = UPDATE.entity('bookshop.Books').alias('books2')
expected.UPDATE.where = [{ list: [{ ref: ['books2', 'ID'] }] }, 'in', subquery]

expect(transformed).to.deep.equal(expected)
})
it('inner joins for the path expression NOT at the leaf of scoped queries, two assocs (UPDATE)', () => {
let query = UPDATE.entity(`bookshop.Authors[books.title = 'bar']:books[genre.parent.name = null]`).alias('MyBook')

const transformed = cqn4sql(query, forNodeModel)
const subquery = cds.ql`
SELECT MyBook.ID from bookshop.Books as MyBook
inner join bookshop.Genres as genre on genre.ID = MyBook.genre_ID
inner join bookshop.Genres as parent on parent.ID = genre.parent_ID
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors
inner join bookshop.Books as books on books.author_ID = Authors.ID
where Authors.ID = MyBook.author_ID and books.title = 'bar'
) and parent.name = null`
const expected = UPDATE.entity('bookshop.Books').alias('MyBook2')
expected.UPDATE.where = [{ list: [{ ref: ['MyBook2', 'ID'] }] }, 'in', subquery]

expect(transformed).to.deep.equal(expected)
})
})
103 changes: 102 additions & 1 deletion db-service/test/cqn4sql/where-exists.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1553,7 +1553,7 @@ describe('path expression within infix filter following exists predicate', () =>
)`,
)
})
it('rejects the path expression at the leaf of scoped queries', () => {
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 All @@ -1567,6 +1567,107 @@ describe('path expression within infix filter following exists predicate', () =>
`Only foreign keys of “genre” can be accessed in infix filter, but found “name”`
)
})
it('renders inner joins for the path expression at the leaf of scoped queries', () => {
let query = CQL`SELECT from bookshop.Authors:books[genre.name = null] { ID }`

const transformed = cqn4sql(query, model)
expect(transformed).to.deep.equal(
CQL`SELECT from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
{ books.ID }
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors where Authors.ID = books.author_ID
)
and genre.name = NULL`,
)
})
it('renders nested inner joins for the path expression at the leaf of scoped queries', () => {
let query = CQL`SELECT from bookshop.Authors:books[genre.parent.name = null] { ID }`
Copy link
Contributor

Choose a reason for hiding this comment

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

What about books that don't have a genre? Should they also appear in the result?


const transformed = cqn4sql(query, model)
expect(transformed).to.deep.equal(
CQL`SELECT from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
inner join bookshop.Genres as parent on parent.ID = genre.parent_ID
{ books.ID }
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors where Authors.ID = books.author_ID
)
and parent.name = NULL`,
)
})
it('renders nested inner joins for the path expression NOT ONLY at the leaf of scoped queries', () => {
// Shadowing is also an issue here.
// should it at all be possible to break out of filter scope (target of assoc)?
let query = CQL`SELECT from bookshop.Authors[books.genre.name = 'Fantasy']:books[genre.parent.name = null] as MyBook { ID }`

const transformed = cqn4sql(query, model)
expect(transformed).to.deep.equal(
CQL`SELECT from bookshop.Books as MyBook
inner join bookshop.Genres as genre on genre.ID = MyBook.genre_ID
inner join bookshop.Genres as parent on parent.ID = genre.parent_ID
{ MyBook.ID }
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors
inner join bookshop.Books as books on books.author_ID = Authors.ID
inner join bookshop.Genres as genre2 on genre2.ID = books.genre_ID
where Authors.ID = MyBook.author_ID and genre2.name = 'Fantasy'
)
and parent.name = NULL`,
)
})
it('renders inner joins for the path expression along the scoped query path', () => {
let query = CQL`SELECT from bookshop.Authors[books.title LIKE '%POE%']:books[genre.name = null] as MyBook { ID }`
// REVISIT:
// if no explicit alias is provided, books.title in the infix filter refers to outer query
// --> no join is generated in that case. How do the statements semantically differ?
const transformed = cqn4sql(query, model)
expect(transformed).to.deep.equal(
CQL`SELECT from bookshop.Books as MyBook
inner join bookshop.Genres as genre on genre.ID = MyBook.genre_ID
{ MyBook.ID }
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors
inner join bookshop.Books as books on books.author_ID = Authors.ID
where Authors.ID = MyBook.author_ID and books.title LIKE '%POE%'
)
and genre.name = NULL`,
)

let queryShadowsAlias = CQL`SELECT from bookshop.Authors[books.title LIKE '%POE%']:books[genre.name = null] { ID }`
const transformedShadowsAlias = cqn4sql(queryShadowsAlias, model)
expect(transformedShadowsAlias).to.deep.equal(
CQL`SELECT from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
{ books.ID }
WHERE EXISTS (
SELECT 1 from bookshop.Authors as Authors
where Authors.ID = books.author_ID and books.title LIKE '%POE%'
)
and genre.name = NULL`,
)
})

it('renders inner joins for the path expression along the scoped query with 3 paths', () => {
let query = CQL`SELECT from bookshop.Authors[books.title LIKE '%POE%']:books[genre.name = null].genre[parent.name = null] as MyGenre { ID }`
const transformed = cqn4sql(query, model)
expect(transformed).to.deep.equal(
CQL`SELECT from bookshop.Genres as MyGenre
inner join bookshop.Genres as parent on parent.ID = MyGenre.parent_ID
{ MyGenre.ID }
WHERE EXISTS (
SELECT 1 from bookshop.Books as books
inner join bookshop.Genres as genre on genre.ID = books.genre_ID
where books.genre_ID = MyGenre.ID and genre.name = null
and EXISTS (
SELECT 1 from bookshop.Authors as Authors
inner join bookshop.Books as books2 on books2.author_ID = Authors.ID
where Authors.ID = books.author_ID and books2.title LIKE '%POE%'
)
)
and parent.name = NULL`,
)
})

it('in case statements', () => {
// TODO: Aliases for genre could be improved
Expand Down
Loading