Skip to content

Commit

Permalink
fix(cqn4sql): be robust against $self.<element> references (#471)
Browse files Browse the repository at this point in the history
if a `ref` of length > 1, has `ref[0] === 'self'`, the `$self` can just
be ignored. `$self` comparisons have only special semantics if compared
to an association. With this change, we become more robust in that
regard and do not prepend wrong table aliases for the case
`$self.<assoc>.<element>` - in that case, we must ensure that the alias
of the association is used and not the alias of the entity. Follow up
#468
  • Loading branch information
patricebender authored Feb 20, 2024
1 parent 41cf4a2 commit 2921b0e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 14 deletions.
22 changes: 14 additions & 8 deletions db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ function cqn4sql(originalQuery, model) {
if (q.SELECT.from.uniqueSubqueryAlias) return
const last = q.SELECT.from.ref.at(-1)
const uniqueSubqueryAlias = inferred.joinTree.addNextAvailableTableAlias(
getLastStringSegment(last.id||last),
getLastStringSegment(last.id || last),
originalQuery.outerQueries,
)
Object.defineProperty(q.SELECT.from, 'uniqueSubqueryAlias', { value: uniqueSubqueryAlias })
Expand Down Expand Up @@ -976,8 +976,7 @@ function cqn4sql(originalQuery, model) {
*/
function isManagedAssocInFlatMode(e) {
return (
e.isAssociation && e.keys
&& (model.meta.transformation === 'odata' || model.meta.unfolded?.includes('structs'))
e.isAssociation && e.keys && (model.meta.transformation === 'odata' || model.meta.unfolded?.includes('structs'))
)
}
}
Expand Down Expand Up @@ -1038,7 +1037,8 @@ function cqn4sql(originalQuery, model) {
let leafAssoc
let element = $refLinks ? $refLinks[$refLinks.length - 1].definition : column
if (isWildcard && element.type === 'cds.LargeBinary') return []
if (element.on && !element.keys) return [] // unmanaged doesn't make it into columns
if (element.on && !element.keys)
return [] // unmanaged doesn't make it into columns
else if (element.virtual === true) return []
else if (!isJoinRelevant && flatName) baseName = flatName
else if (isJoinRelevant) {
Expand Down Expand Up @@ -1276,7 +1276,8 @@ function cqn4sql(originalQuery, model) {
}
} else {
const { list } = token
if (list.every(e => e.val)) // no need for transformation
if (list.every(e => e.val))
// no need for transformation
transformedTokenStream.push({ list })
else transformedTokenStream.push({ list: getTransformedTokenStream(list, $baseLink) })
}
Expand Down Expand Up @@ -1813,8 +1814,12 @@ function cqn4sql(originalQuery, model) {
result.splice(i, 3, ...(wrapInXpr ? [asXpr(backlinkOnCondition)] : backlinkOnCondition))
i += wrapInXpr ? 1 : backlinkOnCondition.length // skip inserted tokens
} else if (lhs.ref) {
if (lhs.ref[0] === '$self') result[i].ref.splice(0, 1, targetSideRefLink.alias)
else if (lhs.ref.length > 1) {
if (lhs.ref[0] === '$self') { // $self in ref of length > 1
// if $self is followed by association, the alias of the association must be used
if (lhs.$refLinks[1].definition.isAssociation) result[i].ref.splice(0, 1)
// otherwise $self is replaced by the alias of the entity
else result[i].ref.splice(0, 1, targetSideRefLink.alias)
} else if (lhs.ref.length > 1) {
if (
!(lhs.ref[0] in pseudos.elements) &&
lhs.ref[0] !== assocRefLink.alias &&
Expand All @@ -1826,7 +1831,8 @@ function cqn4sql(originalQuery, model) {
// first step is the association itself -> use it's name as it becomes the table alias
result[i].ref.splice(0, 1, assocRefLink.alias)
} else if (
definition.name in (targetSideRefLink.definition.elements || targetSideRefLink.definition._target.elements)
definition.name in
(targetSideRefLink.definition.elements || targetSideRefLink.definition._target.elements)
) {
// first step is association which refers to its foreign key by dot notation
result[i].ref = [targetSideRefLink.alias, lhs.ref.join('_')]
Expand Down
2 changes: 1 addition & 1 deletion db-service/test/bookshop/db/schema.cds
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ entity Books : managed {
dedication: String; // same name as struct
};
coAuthor_ID_unmanaged: Integer;
coAuthorUnmanaged: Association to Authors on coAuthorUnmanaged.ID = coAuthor_ID_unmanaged;
coAuthorUnmanaged: Association to Authors on $self.coAuthorUnmanaged.ID = $self.coAuthor_ID_unmanaged;
}

entity BooksWithWeirdOnConditions {
Expand Down
13 changes: 8 additions & 5 deletions db-service/test/cqn4sql/where-exists.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -722,11 +722,7 @@ describe('EXISTS predicate in infix filter', () => {
})
})

/**
* @TODO Review the mean tests and verify, that the resulting cqn 4 sql is valid.
* Especially w.r.t. to table aliases and bracing.
*/
describe('Path in FROM which ends on association must be transformed to where exists', () => {
describe('Scoped queries', () => {
let model
beforeAll(async () => {
model = cds.model = await cds.load(__dirname + '/../bookshop/srv/cat-service').then(cds.linked)
Expand Down Expand Up @@ -817,6 +813,13 @@ describe('Path in FROM which ends on association must be transformed to where ex
WHERE EXISTS ( SELECT 1 from bookshop.Books as Books where Books.author_ID = author.ID
)`)
})
it('unmanaged to one with (multiple) $self in on-condition', () => {
// $self in refs of length > 1 can just be ignored semantically
let query = cqn4sql(CQL`SELECT from bookshop.Books:coAuthorUnmanaged { name }`, model)
expect(query).to.deep.equal(CQL`SELECT from bookshop.Authors as coAuthorUnmanaged { coAuthorUnmanaged.name }
WHERE EXISTS ( SELECT 1 from bookshop.Books as Books where coAuthorUnmanaged.ID = Books.coAuthor_ID_unmanaged
)`)
})
it('handles FROM path with association with explicit table alias', () => {
let query = cqn4sql(CQL`SELECT from bookshop.Books:author as author { author.name }`, model)
expect(query).to.deep.equal(CQL`SELECT from bookshop.Authors as author { author.name }
Expand Down

0 comments on commit 2921b0e

Please sign in to comment.