Skip to content

Commit

Permalink
check also for expand and exists
Browse files Browse the repository at this point in the history
  • Loading branch information
patricebender committed Sep 12, 2024
1 parent 86a70cc commit 033fc07
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 48 deletions.
80 changes: 43 additions & 37 deletions db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const cds = require('@sap/cds')

const infer = require('./infer')
const { computeColumnsToBeSearched } = require('./search')
const { prettyPrintRef } = require('./utils')

/**
* For operators of <eqOps>, this is replaced by comparing all leaf elements with null, combined with and.
Expand Down Expand Up @@ -133,14 +134,14 @@ function cqn4sql(originalQuery, model) {
const keys = Object.values(queryTarget.elements).filter(e => e.key === true)
const primaryKey = { list: [] }
keys
.filter(k => !k.virtual) // e.g. draft column `isActiveEntity` is virtual and key
.forEach(k => {
// cqn4sql will add the table alias to the column later, no need to add it here
subquery.SELECT.columns.push({ ref: [k.name] })
.filter(k => !k.virtual) // e.g. draft column `isActiveEntity` is virtual and key
.forEach(k => {
// cqn4sql will add the table alias to the column later, no need to add it here
subquery.SELECT.columns.push({ ref: [k.name] })

// add the alias of the main query to the list of primary key references
primaryKey.list.push({ ref: [transformedFrom.as, k.name] })
})
// add the alias of the main query to the list of primary key references
primaryKey.list.push({ ref: [transformedFrom.as, k.name] })
})

const transformedSubquery = cqn4sql(subquery, model)

Expand Down Expand Up @@ -765,6 +766,9 @@ function cqn4sql(originalQuery, model) {
function expandColumn(column) {
let outerAlias
let subqueryFromRef
const ref = column.$refLinks[0].definition.kind === 'entity' ? column.ref.slice(1) : column.ref
const assoc = column.$refLinks.at(-1)
ensureValidForeignKeys(assoc.definition, column.ref, 'expand')
if (column.isJoinRelevant) {
// all n-1 steps of the expand column are already transformed into joins
// find the last join relevant association. That is the n-1 assoc in the ref path.
Expand All @@ -784,24 +788,17 @@ function cqn4sql(originalQuery, model) {
outerAlias = transformedQuery.SELECT.from.as
subqueryFromRef = [
...(transformedQuery.SELECT.from.ref || /* subq in from */ [transformedQuery.SELECT.from.target.name]),
...(column.$refLinks[0].definition.kind === 'entity' ? column.ref.slice(1) : column.ref),
...ref,
]
}

// this is the alias of the column which holds the correlated subquery
const columnAlias =
column.as ||
(column.$refLinks[0].definition.kind === 'entity'
? column.ref.slice(1).map(idOnly).join('_') // omit explicit table alias from name of column
: column.ref.map(idOnly).join('_'))
const columnAlias = column.as || ref.map(idOnly).join('_')

// if there is a group by on the main query, all
// columns of the expand must be in the groupBy
if (transformedQuery.SELECT.groupBy) {
const baseRef =
column.$refLinks[0].definition.SELECT || column.$refLinks[0].definition.kind === 'entity'
? column.ref.slice(1)
: column.ref
const baseRef = column.$refLinks[0].definition.SELECT || ref

return _subqueryForGroupBy(column, baseRef, columnAlias)
}
Expand Down Expand Up @@ -1385,6 +1382,8 @@ function cqn4sql(originalQuery, model) {
}”`,
)
}
const { definition: fkSource } = next
ensureValidForeignKeys(fkSource, ref)
whereExistsSubSelects.push(getWhereExistsSubquery(current, next, step.where, true, step.args))
}

Expand Down Expand Up @@ -1586,10 +1585,7 @@ function cqn4sql(originalQuery, model) {
if (!def.$refLinks) return def
const leaf = def.$refLinks[def.$refLinks.length - 1]
const first = def.$refLinks[0]
const tableAlias = getTableAlias(
def,
def.ref.length > 1 && first.definition.isAssociation ? first : $baseLink,
)
const tableAlias = getTableAlias(def, def.ref.length > 1 && first.definition.isAssociation ? first : $baseLink)
if (leaf.definition.parent.kind !== 'entity')
// we need the base name
return getFlatColumnsFor(leaf.definition, {
Expand Down Expand Up @@ -1673,23 +1669,23 @@ function cqn4sql(originalQuery, model) {
const refReverse = [...from.ref].reverse()
const $refLinksReverse = [...transformedFrom.$refLinks].reverse()
for (let i = 0; i < refReverse.length; i += 1) {
const stepLink = $refLinksReverse[i]
const current = $refLinksReverse[i]

let nextStepLink = $refLinksReverse[i + 1]
let next = $refLinksReverse[i + 1]
const nextStep = refReverse[i + 1] // only because we want the filter condition

if (stepLink.definition.target && nextStepLink) {
if (current.definition.target && next) {
const { where, args } = nextStep
if (isStructured(nextStepLink.definition)) {
if (isStructured(next.definition)) {
// find next association / entity in the ref because this is actually our real nextStep
const nextStepIndex =
2 +
$refLinksReverse
.slice(i + 2)
.findIndex(rl => rl.definition.isAssociation || rl.definition.kind === 'entity')
nextStepLink = $refLinksReverse[nextStepIndex]
next = $refLinksReverse[nextStepIndex]
}
let as = getLastStringSegment(nextStepLink.alias)
let as = getLastStringSegment(next.alias)
/**
* for an `expand` subquery, we do not need to add
* the table alias of the `expand` host to the join tree
Expand All @@ -1699,8 +1695,10 @@ function cqn4sql(originalQuery, model) {
if (!(inferred.SELECT?.expand === true)) {
as = getNextAvailableTableAlias(as)
}
nextStepLink.alias = as
whereExistsSubSelects.push(getWhereExistsSubquery(stepLink, nextStepLink, where, false, args))
next.alias = as
const { definition: fkSource } = current
ensureValidForeignKeys(fkSource, from.ref)
whereExistsSubSelects.push(getWhereExistsSubquery(current, next, where, false, args))
}
}

Expand Down Expand Up @@ -1745,6 +1743,14 @@ function cqn4sql(originalQuery, model) {
}
}

function ensureValidForeignKeys(fkSource, ref, kind = null) {
if (fkSource.keys && fkSource.keys.length === 0) {
const path = prettyPrintRef(ref, model)
if (kind === 'expand') throw new Error(`Can't expand “${fkSource.name}” as it has no foreign keys`)
throw new Error(`Path step “${fkSource.name}” of “${path}” has no foreign keys`)
}
}

function whereExistsSubqueries(whereExistsSubSelects) {
if (whereExistsSubSelects.length === 1) return whereExistsSubSelects[0]
whereExistsSubSelects.reduce((prev, cur) => {
Expand Down Expand Up @@ -1855,7 +1861,7 @@ function cqn4sql(originalQuery, model) {
result[i] = asXpr(xpr)
continue
}
if(lhs.args) {
if (lhs.args) {
const args = calculateOnCondition(lhs.args)
result[i] = { ...lhs, args }
continue
Expand Down Expand Up @@ -2272,13 +2278,6 @@ function cqn4sql(originalQuery, model) {
}
}

module.exports = Object.assign(cqn4sql, {
// for own tests only:
eqOps,
notEqOps,
notSupportedOps,
})

function calculateElementName(token) {
const nonJoinRelevantAssoc = [...token.$refLinks].findIndex(l => l.definition.isAssociation && l.onlyForeignKeyAccess)
let name
Expand Down Expand Up @@ -2366,3 +2365,10 @@ const refWithConditions = step => {
return appendix ? step.id + appendix : step
}
const is_regexp = x => x?.constructor?.name === 'RegExp' // NOTE: x instanceof RegExp doesn't work in repl

module.exports = Object.assign(cqn4sql, {
// for own tests only:
eqOps,
notEqOps,
notSupportedOps,
})
7 changes: 3 additions & 4 deletions db-service/lib/infer/join-tree.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict'

const { prettyPrintRef } = require('../utils')

// REVISIT: define following unknown types

/**
Expand Down Expand Up @@ -186,10 +188,7 @@ class JoinTree {
const $refLink = col.$refLinks[i]
// sanity check: error out if we can't produce a join
if ($refLink.definition.keys && $refLink.definition.keys.length === 0) {
const path = col.ref.reduce((acc, curr, j) => {
if (j > 0) acc += '.'
return acc + `${curr.id ? curr.id + '[…]' : curr}`
}, '')
const path = prettyPrintRef(col.ref)
throw new Error(`Path step “${$refLink.alias}” of “${path}” has no valid foreign keys`)
}

Expand Down
27 changes: 27 additions & 0 deletions db-service/lib/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict'

/**
* Formats a ref array into a string representation.
* If the first step is an entity, the separator is a colon, otherwise a dot.
*
* @param {Array} ref - The reference array to be formatted.
* @param {Object} model - The model object containing definitions.
* @returns {string} The formatted string representation of the reference.
*/
function prettyPrintRef(ref, model = null) {
return ref.reduce((acc, curr, j) => {
if (j > 0) {
if (j === 1 && model?.definitions[ref[0]]?.kind === 'entity') {
acc += ':'
} else {
acc += '.'
}
}
return acc + `${curr.id ? curr.id + '[…]' : curr}`
}, '')
}

// export the function to be used in other modules
module.exports = {
prettyPrintRef,
}
42 changes: 35 additions & 7 deletions db-service/test/cqn4sql/keyless.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe('keyless entities', () => {
expect(() => cqn4sql(q, model)).to.throw(
'Path step “author” of “author[…].book[…].author.name” has no valid foreign keys',
)
// ok if explicit foreign key is used
const qOk = SELECT.columns('ID').from(Books).where(`authorWithExplicitForeignKey[ID = 42].name LIKE 'King'`)
expect(cqn4sql(qOk, model)).to.eql(
CQL`SELECT Books.ID FROM Books as Books
Expand All @@ -28,17 +29,44 @@ describe('keyless entities', () => {
where authorWithExplicitForeignKey.name LIKE 'King'`,
)
})

it.skip('scoped query leading to where exists subquery cant be constructed', () => {
it('scoped query leading to where exists subquery cant be constructed', () => {
const q = SELECT.from('Books:author')
expect(() => cqn4sql(q, model)).to.throw()
expect(() => cqn4sql(q, model)).to.throw(`Path step “author” of “Books:author” has no foreign keys`)

// ok if explicit foreign key is used
const qOk = SELECT.from('Books:authorWithExplicitForeignKey').columns('ID')
expect(cqn4sql(qOk, model)).to.eql(
CQL`SELECT authorWithExplicitForeignKey.ID FROM Authors as authorWithExplicitForeignKey
where exists (
SELECT 1 from Books as Books where Books.authorWithExplicitForeignKey_ID = authorWithExplicitForeignKey.ID
)`,
)
})
it.skip('where exists predicate cant be transformed to subquery', () => {
it('where exists predicate cant be transformed to subquery', () => {
const q = SELECT.from('Books').where('exists author')
expect(() => cqn4sql(q, model)).to.throw()
expect(() => cqn4sql(q, model)).to.throw(`Path step “author” of “author” has no foreign keys`)
// ok if explicit foreign key is used
const qOk = SELECT.from('Books').columns('ID').where('exists authorWithExplicitForeignKey')
expect(cqn4sql(qOk, model)).to.eql(
CQL`SELECT Books.ID FROM Books as Books
where exists (
SELECT 1 from Authors as authorWithExplicitForeignKey where authorWithExplicitForeignKey.ID = Books.authorWithExplicitForeignKey_ID
)`,
)
})
it.skip('correlated subquery for expand cant be constructed', () => {
it('correlated subquery for expand cant be constructed', () => {
const q = CQL`SELECT author { name } from Books`
expect(() => cqn4sql(q, model)).to.throw()
expect(() => cqn4sql(q, model)).to.throw(`Can't expand “author” as it has no foreign keys`)
// ok if explicit foreign key is used
const qOk = CQL`SELECT authorWithExplicitForeignKey { name } from Books`
expect(JSON.parse(JSON.stringify(cqn4sql(qOk, model)))).to.eql(
CQL`
SELECT
(
SELECT authorWithExplicitForeignKey.name from Authors as authorWithExplicitForeignKey
where Books.authorWithExplicitForeignKey_ID = authorWithExplicitForeignKey.ID
) as authorWithExplicitForeignKey
from Books as Books`,
)
})
})

0 comments on commit 033fc07

Please sign in to comment.