Skip to content

Commit

Permalink
feat: enable path expressions in infix filter after exists predicate (
Browse files Browse the repository at this point in the history 
#875)

if a path expression is detected in a infix filter of an association
which follows an `exists` predicate, we add the filter expression as a
whole to the `exists (<subquery>)` and then recursively transform it
with `cqn4sql` to get the proper joins.

Usually, transforming the `exists <subquery>` is not necessary, because
the `where` clause is already well formed -> there is a mechanism to
flag a path expression in this special case as such.

this is just a simple poc… more tests need to be added.

- [x] `exists books[genre.name = 'fiction' and exists author]` -> mixed
with sibiling `exists`
- [x] `exists books[uppercase(genre.name) = 'FICTION']` -> detect path
expressions anywhere
- [x] `SELECT from Authors:books[genre.name = 'FICTION']` → What about
scoped queries?
  + we dont tacke them, yet

fix: nested exists wrapped in xpr
  reported in cap/cdsnode/issues/2194
  • Loading branch information
@patricebender
patricebender authored Nov 13, 2024
1 parent 6684436 commit 7e50359
Show file tree
Hide file tree
Showing 7 changed files with 321 additions and 60 deletions.
31 changes: 23 additions & 8 deletions db-service/lib/cqn4sql.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,10 @@ function cqn4sql(originalQuery, model) {
const id = localized(r.queryArtifact)
args.push({ ref: [r.args ? { id, args: r.args } : id], as: r.alias })
}
from = { join: 'left', args, on: [] }
from = { join: r.join || 'left', args, on: [] }
r.children.forEach(c => {
from = joinForBranch(from, c)
from = { join: 'left', args: [from], on: [] }
from = { join: c.join || 'left', args: [from], on: [] }
})
})
return from.args.length > 1 ? from : from.args[0]
Expand Down Expand Up @@ -309,7 +309,7 @@ function cqn4sql(originalQuery, model) {
}
if (node.children) {
node.children.forEach(c => {
lhs = { join: 'left', args: [lhs], on: [] }
lhs = { join: c.join || 'left', args: [lhs], on: [] }
lhs = joinForBranch(lhs, c)
})
}
Expand Down Expand Up @@ -2093,11 +2093,6 @@ function cqn4sql(originalQuery, model) {
const unmanagedOn = onCondFor(inWhere ? next : current, inWhere ? current : next, inWhere)
on.push(...(customWhere && hasLogicalOr(unmanagedOn) ? [asXpr(unmanagedOn)] : unmanagedOn))
}
// infix filter conditions are wrapped in `xpr` when added to the on-condition
if (customWhere) {
const filter = getTransformedTokenStream(customWhere, next)
on.push(...['and', ...(hasLogicalOr(filter) ? [asXpr(filter)] : filter)])
}

const subquerySource = assocTarget(nextDefinition) || nextDefinition
const id = localized(subquerySource)
Expand All @@ -2115,6 +2110,26 @@ function cqn4sql(originalQuery, model) {
],
where: on,
}
if (next.pathExpressionInsideFilter) {
SELECT.where = customWhere
const transformedExists = transformSubquery({ SELECT })
// infix filter conditions are wrapped in `xpr` when added to the on-condition
if (transformedExists.SELECT.where) {
on.push(
...[
'and',
...(hasLogicalOr(transformedExists.SELECT.where)
? [asXpr(transformedExists.SELECT.where)]
: transformedExists.SELECT.where),
],
)
}
transformedExists.SELECT.where = on
return transformedExists.SELECT
} else if (customWhere) {
const filter = getTransformedTokenStream(customWhere, next)
on.push(...['and', ...(hasLogicalOr(filter) ? [asXpr(filter)] : filter)])
}
return SELECT
}

Expand Down
93 changes: 55 additions & 38 deletions db-service/lib/infer/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,16 +184,13 @@ function infer(originalQuery, model) {
if (e.target) {
// only fk access in infix filter
const nextStep = ref[1]?.id || ref[1]
// no unmanaged assoc in infix filter path
if (!expandOrExists && e.on) {
const err = `Unexpected unmanaged association “${e.name}” in filter expression of “${$baseLink.definition.name}”`
throw new Error(err)
if (isNonForeignKeyNavigation(e, nextStep)) {
if (expandOrExists) {
Object.defineProperty($baseLink, 'pathExpressionInsideFilter', { value: true })
} else {
rejectNonFkNavigation(e, e.on ? $baseLink.definition.name : nextStep)
}
}
// no non-fk traversal in infix filter
if (!expandOrExists && nextStep && !isForeignKeyOf(nextStep, e))
throw new Error(
`Only foreign keys of “${e.name}” can be accessed in infix filter, but found “${nextStep}”`,
)
}
arg.$refLinks.push({ definition: e, target: definition })
// filter paths are flattened
Expand Down Expand Up @@ -226,7 +223,7 @@ function infer(originalQuery, model) {
// don't miss an exists within an expression
token.xpr.forEach(walkTokenStream)
} else {
attachRefLinksToArg(token, arg.$refLinks[i], existsPredicate)
attachRefLinksToArg(token, arg.$refLinks[i], existsPredicate || expandOrExists)
existsPredicate = false
}
}
Expand All @@ -235,6 +232,7 @@ function infer(originalQuery, model) {
}
i += 1
}
if ($baseLink?.pathExpressionInsideFilter) Object.defineProperty(arg, 'join', { value: 'inner' })
const { definition, target } = arg.$refLinks[arg.$refLinks.length - 1]
if (definition.value) {
// nested calculated element
Expand Down Expand Up @@ -542,9 +540,19 @@ function infer(originalQuery, model) {
const elements = getDefinition(definition.target)?.elements || definition.elements
if (elements && id in elements) {
const element = elements[id]
rejectNonFkAccess(element)
if (inInfixFilter) {
const nextStep = column.ref[1]?.id || column.ref[1]
if (isNonForeignKeyNavigation(element, nextStep)) {
if (inExists) {
Object.defineProperty($baseLink, 'pathExpressionInsideFilter', { value: true })
} else {
rejectNonFkNavigation(element, element.on ? $baseLink.definition.name : nextStep)
}
}
}
const resolvableIn = getDefinition(definition.target) || target
column.$refLinks.push({ definition: elements[id], target: resolvableIn })
const $refLink = { definition: elements[id], target: resolvableIn }
column.$refLinks.push($refLink)
} else {
stepNotFoundInPredecessor(id, definition.name)
}
Expand Down Expand Up @@ -593,7 +601,16 @@ function infer(originalQuery, model) {

const target = getDefinition(definition.target) || column.$refLinks[i - 1].target
if (element) {
if ($baseLink) rejectNonFkAccess(element)
if ($baseLink && inInfixFilter) {
const nextStep = column.ref[i + 1]?.id || column.ref[i + 1]
if (isNonForeignKeyNavigation(element, nextStep)) {
if (inExists) {
Object.defineProperty($baseLink, 'pathExpressionInsideFilter', { value: true })
} else {
rejectNonFkNavigation(element, element.on ? $baseLink.definition.name : nextStep)
}
}
}
const $refLink = { definition: elements[id], target }
column.$refLinks.push($refLink)
} else if (firstStepIsSelf) {
Expand Down Expand Up @@ -637,7 +654,7 @@ function infer(originalQuery, model) {
skipJoinsForFilter = true
} else if (token.ref || token.xpr) {
inferQueryElement(token, false, column.$refLinks[i], {
inExists: skipJoinsForFilter,
inExists: skipJoinsForFilter || inExists,
inExpr: !!token.xpr,
inInfixFilter: true,
})
Expand All @@ -646,7 +663,7 @@ function infer(originalQuery, model) {
applyToFunctionArgs(token.args, inferQueryElement, [
false,
column.$refLinks[i],
{ inExists: skipJoinsForFilter, inExpr: true, inInfixFilter: true },
{ inExists: skipJoinsForFilter || inExists, inExpr: true, inInfixFilter: true },
])
}
}
Expand Down Expand Up @@ -700,31 +717,11 @@ function infer(originalQuery, model) {
}
}
}

/**
* Check if the next step in the ref is foreign key of `assoc`
* if not, an error is thrown.
*
* @param {CSN.Element} assoc if this is an association, the next step must be a foreign key of the element.
*/
function rejectNonFkAccess(assoc) {
if (inInfixFilter && assoc.target) {
// only fk access in infix filter
const nextStep = column.ref[i + 1]?.id || column.ref[i + 1]
// no unmanaged assoc in infix filter path
if (!inExists && assoc.on) {
const err = `Unexpected unmanaged association “${assoc.name}” in filter expression of “${$baseLink.definition.name}”`
throw new Error(err)
}
// no non-fk traversal in infix filter in non-exists path
if (nextStep && !assoc.on && !isForeignKeyOf(nextStep, assoc))
throw new Error(
`Only foreign keys of “${assoc.name}” can be accessed in infix filter, but found “${nextStep}”`,
)
}
}
})

// we need inner joins for the path expressions inside filter expressions after exists predicate
if ($baseLink?.pathExpressionInsideFilter) Object.defineProperty(column, 'join', { value: 'inner' })

// ignore whole expand if target of assoc along path has ”@cds.persistence.skip”
if (column.expand) {
const { $refLinks } = column
Expand Down Expand Up @@ -1214,6 +1211,26 @@ function infer(originalQuery, model) {
}
}

/**
* Determines if a given association is a non-foreign key navigation.
*
* @param {Object} assoc - The association.
* @param {Object} nextStep - The next step in the navigation path.
* @returns {boolean} - Returns true if the next step is a non-foreign key navigation, otherwise false.
*/
function isNonForeignKeyNavigation(assoc, nextStep) {
if (!nextStep || !assoc.target) return false

return assoc.on || !isForeignKeyOf(nextStep, assoc)
}

function rejectNonFkNavigation(assoc, additionalInfo) {
if (assoc.on) {
throw new Error(`Unexpected unmanaged association “${assoc.name}” in filter expression of “${additionalInfo}”`)
}
throw new Error(`Only foreign keys of “${assoc.name}” can be accessed in infix filter, but found “${additionalInfo}”`)
}

/**
* Returns true if e is a foreign key of assoc.
* this function is also compatible with unfolded csn (UCSN),
Expand Down
1 change: 1 addition & 0 deletions db-service/lib/infer/join-tree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ class JoinTree {
// if no root node was found, the column is selected from a subquery
if (!node) return
while (i < col.ref.length) {
if(col.join === 'inner') node.join = 'inner'
const step = col.ref[i]
const { where, args } = step
const id = joinId(step, args, where)
Expand Down
16 changes: 11 additions & 5 deletions db-service/test/cds-infer/negative.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,17 +399,23 @@ describe('negative', () => {
/Only foreign keys of “author” can be accessed in infix filter, but found “name”/,
)
})
it('rejects non fk traversal in infix filter in where exists', () => {
it('does not reject non fk traversal in infix filter in where exists', () => {
let query = CQL`SELECT from bookshop.Books where exists author.books[author.name = 'John Doe']`
expect(() => _inferred(query)).to.not.throw(
/Only foreign keys of “author” can be accessed in infix filter, but found “name”/,
)
})
it('rejects non fk traversal in infix filter in where', () => {
let query = CQL`SELECT from bookshop.Books where author.books[author.name = 'John Doe'].title = 'foo'`
expect(() => _inferred(query)).to.throw(
/Only foreign keys of “author” can be accessed in infix filter, but found “name”/,
) // revisit: better error location ""bookshop.Books:author"
)
})
it('rejects unmanaged traversal in infix filter in where exists', () => {
it('does not reject unmanaged traversal in infix filter in where exists', () => {
let query = CQL`SELECT from bookshop.Books where exists author.books[coAuthorUnmanaged.name = 'John Doe']`
expect(() => _inferred(query)).to.throw(
expect(() => _inferred(query)).to.not.throw(
/Unexpected unmanaged association “coAuthorUnmanaged” in filter expression of “books”/,
) // revisit: better error location ""bookshop.Books:author"
)
})

it('rejects non fk traversal in infix filter in column', () => {
Expand Down
Loading

0 comments on commit 7e50359

Please sign in to comment.