fix: reject all path expressions w/o foreign keys

db-service/lib/cqn4sql.js

@@ -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.
@@ -762,6 +763,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.
@@ -781,24 +785,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)
  }
@@ -1384,6 +1381,8 @@ function cqn4sql(originalQuery, model) {
  }”`,
  )
  }
+ const { definition: fkSource } = next
+ ensureValidForeignKeys(fkSource, ref)
  whereExistsSubSelects.push(getWhereExistsSubquery(current, next, step.where, true, step.args))
  }
 
@@ -1669,23 +1668,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
@@ -1695,8 +1694,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))
  }
  }
 
@@ -1741,6 +1742,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) => {
@@ -1817,7 +1826,6 @@ function cqn4sql(originalQuery, model) {
  const { on, keys } = assocRefLink.definition
  const target = getDefinition(assocRefLink.definition.target)
  let res
- // technically we could have multiple backlinks
  if (keys) {
  const fkPkPairs = getParentKeyForeignKeyPairs(assocRefLink.definition, targetSideRefLink, true)
  const transformedOn = []
@@ -1950,6 +1958,12 @@ function cqn4sql(originalQuery, model) {
  }
  })
  } else if (backlink.keys) {
+ // sanity check: error out if we can't produce a join
+ if (backlink.keys.length === 0) {
+ throw new Error(
+ `Path step “${assocRefLink.alias}” is a self comparison with “${getFullName(backlink)}” that has no foreign keys`,
+ )
+ }
  // managed backlink -> calculate fk-pk pairs
  const fkPkPairs = getParentKeyForeignKeyPairs(backlink, targetSideRefLink)
  fkPkPairs.forEach((pair, j) => {
@@ -2268,13 +2282,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
@@ -2362,3 +2369,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,
+})

db-service/lib/infer/join-tree.js

@@ -1,5 +1,7 @@
 'use strict'
 
+const { prettyPrintRef } = require('../utils')
+
 // REVISIT: define following unknown types
 
 /**
@@ -177,13 +179,19 @@ class JoinTree {
  }
 
  // if no root node was found, the column is selected from a subquery
- if(!node) return
+ if (!node) return
  while (i < col.ref.length) {
  const step = col.ref[i]
  const { where, args } = step
  const id = joinId(step, args, where)
  const next = node.children.get(id)
  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 = prettyPrintRef(col.ref)
+ throw new Error(`Path step “${$refLink.alias}” of “${path}” has no foreign keys`)
+ }
+
  if (next) {
  // step already seen before
  node = next
@@ -208,7 +216,6 @@ class JoinTree {
  }
  child.$refLink.alias = this.addNextAvailableTableAlias($refLink.alias, outerQueries)
  }
- //> REVISIT: remove fallback once UCSN is standard
  const elements =
  node.$refLink?.definition.isAssociation &&
  (node.$refLink.definition.elements || node.$refLink.definition.foreignKeys)

db-service/lib/utils.js

@@ -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,
+}

db-service/test/cqn4sql/keyless.test.js

@@ -0,0 +1,107 @@
+/**
+ * Make sure we issue proper errors in case of path expression along keyless entities
+ */
+'use strict'
+
+const cqn4sql = require('../../lib/cqn4sql')
+const cds = require('@sap/cds')
+const { expect } = cds.test
+describe('keyless entities', () => {
+ let model
+
+ beforeAll(async () => {
+ model = await cds.load(__dirname + '/model/keyless').then(cds.linked)
+ })
+ describe('managed assocs', () => {
+ it('no foreign keys for join', () => {
+ const { Books } = model.entities
+ const q = SELECT.from(Books).where(`author[ID = 42].book[ID = 42].author.name LIKE 'King'`)
+ expect(() => cqn4sql(q, model)).to.throw(
+ 'Path step “author” of “author[…].book[…].author.name” has no 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 
+ left join Authors as authorWithExplicitForeignKey
+ on authorWithExplicitForeignKey.ID = Books.authorWithExplicitForeignKey_ID
+ and authorWithExplicitForeignKey.ID = 42
+ where authorWithExplicitForeignKey.name LIKE 'King'`,
+ )
+ })
+ it('no foreign keys for join (2)', () => {
+ const { Authors } = model.entities
+ const q = SELECT.from(Authors).where(`book.authorWithExplicitForeignKey.book.my.author LIKE 'King'`)
+ expect(() => cqn4sql(q, model)).to.throw(
+ 'Path step “author” of “book.authorWithExplicitForeignKey.book.my.author” has no foreign keys',
+ )
+ })
+ it('scoped query leading to where exists subquery cant be constructed', () => {
+ const q = SELECT.from('Books:author')
+ 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('where exists predicate cant be transformed to subquery', () => {
+ const q = SELECT.from('Books').where('exists author')
+ 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('correlated subquery for expand cant be constructed', () => {
+ const q = CQL`SELECT author { name } from Books`
+ 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`,
+ )
+ })
+ })
+ describe('managed assocs as backlinks', () => {
+ it('backlink has no foreign keys for join', () => {
+ const { Authors } = model.entities
+ const q = SELECT.from(Authors).where(`bookWithBackLink.title LIKE 'Potter'`)
+ expect(() => cqn4sql(q, model)).to.throw(
+ `Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`,
+ )
+ })
+ it('backlink has no foreign keys for scoped query', () => {
+ const q = SELECT.from('Authors:bookWithBackLink')
+ expect(() => cqn4sql(q, model)).to.throw(
+ `Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`,
+ )
+ })
+ it('backlink has no foreign keys for where exists subquery', () => {
+ const q = SELECT.from('Authors').where('exists bookWithBackLink')
+ expect(() => cqn4sql(q, model)).to.throw(
+ `Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`,
+ )
+ })
+ it('backlink has no foreign keys for expand subquery', () => {
+ const q = CQL`SELECT bookWithBackLink { title } from Authors`
+ expect(() => cqn4sql(q, model)).to.throw(
+ `Path step “bookWithBackLink” is a self comparison with “author” that has no foreign keys`,
+ )
+ })
+ })
+})

db-service/test/cqn4sql/model/keyless.cds

@@ -0,0 +1,17 @@
+// path expressions along `Authors:author` are not possible
+entity Books {
+ key ID : Integer;
+ title : String;
+ stock : Integer;
+ author : Association to Authors;
+ authorWithExplicitForeignKey: Association to Authors { ID };
+ my: Association to Books;
+}
+
+entity Authors {
+ ID : Integer;
+ name : String;
+ book: Association to Books;
+ // backlink has no foreign keys...
+ bookWithBackLink: Association to Books on bookWithBackLink.author = $self;
+}