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

fix: assign more technical, implicit table aliases #1082

Merged
merged 37 commits into from
Mar 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
80f20ba
fix: technical table alias
patricebender Mar 12, 2025
5198438
migrated first test suite
patricebender Mar 13, 2025
8c1e06a
migrated another test suite
patricebender Mar 13, 2025
5f06eb2
migrated another test suite
patricebender Mar 13, 2025
b0f9a00
migrated another test suite
patricebender Mar 13, 2025
b985605
migrated another test suite
patricebender Mar 13, 2025
eb95078
migrated another test suite
patricebender Mar 14, 2025
d7cd649
migrated another test suite
patricebender Mar 14, 2025
bbfcba5
migrated another test suite
patricebender Mar 14, 2025
9bff27c
migrated another test suite
patricebender Mar 14, 2025
6d16143
migrated another test suite
patricebender Mar 14, 2025
93df3cc
migrated another test suite
patricebender Mar 14, 2025
e5b10cc
migrated another test suite
patricebender Mar 14, 2025
8c37863
migrated another test suite
patricebender Mar 14, 2025
a856539
migrated another test suite
patricebender Mar 14, 2025
1f88d42
migrated another test suite
patricebender Mar 15, 2025
9f8b0ed
Merge remote-tracking branch 'origin/main' into patrice/technical-alias
patricebender Mar 21, 2025
807c88a
migrated another test suite
patricebender Mar 21, 2025
abf40bf
migrated another test suite
patricebender Mar 21, 2025
5cac5a7
migrated another test suite
patricebender Mar 25, 2025
96b8cae
migrated another test suite
patricebender Mar 25, 2025
a9c0fb0
migrated another test suite
patricebender Mar 25, 2025
dcf8736
migrated another test suite
patricebender Mar 25, 2025
5643c30
migrated another test suite
patricebender Mar 25, 2025
2b01ae9
migrated another test suite
patricebender Mar 26, 2025
7cab676
migrate the last test suite
patricebender Mar 26, 2025
c2b15b5
adapt another reference
patricebender Mar 27, 2025
7a881ef
Merge remote-tracking branch 'origin/main' into patrice/technical-alias
patricebender Mar 27, 2025
a758468
Merge branch 'main' into patrice/technical-alias
johannes-vogel Mar 28, 2025
34b6ceb
fix ambigous alias for expand
patricebender Mar 28, 2025
7fb2fff
pass model to cqn4sql to avoid race condition through cds-test
patricebender Mar 28, 2025
077042f
Merge remote-tracking branch 'origin/main' into patrice/technical-alias
patricebender Mar 28, 2025
33e3a55
for multiple query sources, no technical alias
patricebender Mar 28, 2025
92d7d3f
some tests for special entity names
patricebender Mar 28, 2025
56ddf0a
run all tests
patricebender Mar 28, 2025
627e605
Merge branch 'main' into patrice/technical-alias
johannes-vogel Mar 31, 2025
7651941
Merge branch 'main' into patrice/technical-alias
johannes-vogel Mar 31, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 74 additions & 71 deletions db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ cds.infer.target ??= q => q._target || q.target // instanceof cds.entity ? q._ta

const infer = require('./infer')
const { computeColumnsToBeSearched } = require('./search')
const { prettyPrintRef, isCalculatedOnRead, isCalculatedElement } = require('./utils')
const { prettyPrintRef, isCalculatedOnRead, isCalculatedElement, getImplicitAlias } = require('./utils')

/**
* For operators of <eqOps>, this is replaced by comparing all leaf elements with null, combined with and.
Expand Down Expand Up @@ -63,16 +63,16 @@ function cqn4sql(originalQuery, model) {
}
// query modifiers can also be defined in from ref leaf infix filter
// > SELECT from bookshop.Books[order by price] {ID}
if(inferred.SELECT?.from.ref) {
for(const [key, val] of Object.entries(inferred.SELECT.from.ref.at(-1))) {
if(key in { orderBy: 1, groupBy: 1 }) {
if(inferred.SELECT[key]) inferred.SELECT[key].push(...val)
if (inferred.SELECT?.from.ref) {
for (const [key, val] of Object.entries(inferred.SELECT.from.ref.at(-1))) {
if (key in { orderBy: 1, groupBy: 1 }) {
if (inferred.SELECT[key]) inferred.SELECT[key].push(...val)
else inferred.SELECT[key] = val
} else if(key === 'limit') {
} else if (key === 'limit') {
// limit defined on the query has precedence
if(!inferred.SELECT.limit) inferred.SELECT.limit = val
} else if(key === 'having') {
if(!inferred.SELECT.having) inferred.SELECT.having = val
if (!inferred.SELECT.limit) inferred.SELECT.limit = val
} else if (key === 'having') {
if (!inferred.SELECT.having) inferred.SELECT.having = val
else inferred.SELECT.having.push('and', ...val)
}
}
Expand Down Expand Up @@ -818,16 +818,18 @@ function cqn4sql(originalQuery, model) {
return _subqueryForGroupBy(column, baseRef, columnAlias)
}

// we need to respect the aliases of the outer query, so the columnAlias might not be suitable
// as table alias for the correlated subquery
const uniqueSubqueryAlias = getNextAvailableTableAlias(columnAlias, inferred.outerQueries)
// Alias in expand subquery is derived from but not equal to
// the alias of the column because to account for potential ambiguities
// the alias cannot be addressed anyways
const uniqueSubqueryAlias = getNextAvailableTableAlias(getImplicitAlias(columnAlias), inferred.outerQueries)

// `SELECT from Authors { books.genre as genreOfBooks { name } } becomes `SELECT from Books:genre as genreOfBooks`
const from = { ref: subqueryFromRef, as: uniqueSubqueryAlias }
const subqueryBase = {}
const queryModifiers = { ...column }
for (const [key, value] of Object.entries(queryModifiers)) {
if (key in { limit: 1, orderBy: 1, groupBy: 1, excluding: 1, where: 1, having: 1, count: 1 }) subqueryBase[key] = value
if (key in { limit: 1, orderBy: 1, groupBy: 1, excluding: 1, where: 1, having: 1, count: 1 })
subqueryBase[key] = value
}

const subquery = {
Expand Down Expand Up @@ -871,6 +873,7 @@ function cqn4sql(originalQuery, model) {
const existsSubqueryAlias = recent[existsIndex + 1].SELECT.from.as
if (existsSubqueryAlias === x.ref?.[0]) return { ref: [outer, ...x.ref.slice(1)] }
if (x.xpr) x.xpr = x.xpr.map(replaceAliasWithSubqueryAlias)
if (x.args) x.args = x.args.map(replaceAliasWithSubqueryAlias)
return x
}
return subq
Expand Down Expand Up @@ -900,37 +903,39 @@ function cqn4sql(originalQuery, model) {
// expand with wildcard vanishes as expand is part of the group by (OData $apply + $expand)
return null
}
const expandedColumns = column.expand.flatMap(expand => {
if (!expand.ref) return expand
const fullRef = [...baseRef, ...expand.ref]

if (expand.expand) {
const nested = _subqueryForGroupBy(expand, fullRef, expand.as || expand.ref.map(idOnly).join('_'))
if(nested) {
setElementOnColumns(nested, expand.element)
elements[expand.as || expand.ref.map(idOnly).join('_')] = nested
const expandedColumns = column.expand
.flatMap(expand => {
if (!expand.ref) return expand
const fullRef = [...baseRef, ...expand.ref]

if (expand.expand) {
const nested = _subqueryForGroupBy(expand, fullRef, expand.as || expand.ref.map(idOnly).join('_'))
if (nested) {
setElementOnColumns(nested, expand.element)
elements[expand.as || expand.ref.map(idOnly).join('_')] = nested
}
return nested
}
return nested
}

const groupByRef = groupByLookup.get(fullRef.map(refWithConditions).join('.'))
if (!groupByRef) {
throw new Error(
`The expanded column "${fullRef.map(refWithConditions).join('.')}" must be part of the group by clause`,
)
}
const groupByRef = groupByLookup.get(fullRef.map(refWithConditions).join('.'))
if (!groupByRef) {
throw new Error(
`The expanded column "${fullRef.map(refWithConditions).join('.')}" must be part of the group by clause`,
)
}

const copy = Object.create(groupByRef)
// always alias for this special case, so that they nested element names match the expected result structure
// otherwise we'd get `author { <outer>.author_ID }`, but we need `author { <outer>.author_ID as ID }`
copy.as = expand.as || expand.ref.at(-1)
const tableAlias = getTableAlias(copy)
const res = getFlatColumnsFor(copy, { tableAlias })
res.forEach(c => {
elements[c.as || c.ref.at(-1)] = c.element
const copy = Object.create(groupByRef)
// always alias for this special case, so that they nested element names match the expected result structure
// otherwise we'd get `author { <outer>.author_ID }`, but we need `author { <outer>.author_ID as ID }`
copy.as = expand.as || expand.ref.at(-1)
const tableAlias = getTableAlias(copy)
const res = getFlatColumnsFor(copy, { tableAlias })
res.forEach(c => {
elements[c.as || c.ref.at(-1)] = c.element
})
return res
})
return res
}).filter(c => c)
.filter(c => c)

if (expandedColumns.length === 0) {
return null
Expand Down Expand Up @@ -1074,7 +1079,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),
getImplicitAlias(last.id || last),
inferred.outerQueries,
)
Object.defineProperty(q.SELECT.from, 'uniqueSubqueryAlias', { value: uniqueSubqueryAlias })
Expand Down Expand Up @@ -1403,7 +1408,7 @@ function cqn4sql(originalQuery, model) {
j = nextAssocIndex
}

const as = getNextAvailableTableAlias(getLastStringSegment(next.alias))
const as = getNextAvailableTableAlias(getImplicitAlias(next.alias))
next.alias = as
if (!next.definition.target) {
let type = next.definition.type
Expand Down Expand Up @@ -1699,12 +1704,12 @@ function cqn4sql(originalQuery, model) {
function _transformFrom() {
if (typeof from === 'string') {
// normalize to `ref`, i.e. for `UPDATE.entity('bookshop.Books')`
return { transformedFrom: { ref: [from], as: getLastStringSegment(from) } }
return { transformedFrom: { ref: [from], as: getImplicitAlias(from) } }
}
transformedFrom.as =
from.uniqueSubqueryAlias ||
from.as ||
getLastStringSegment(transformedFrom.$refLinks[transformedFrom.$refLinks.length - 1].definition.name)
getImplicitAlias(transformedFrom.$refLinks[transformedFrom.$refLinks.length - 1].definition.name)
const whereExistsSubSelects = []
const filterConditions = []
const refReverse = [...from.ref].reverse()
Expand All @@ -1726,14 +1731,17 @@ function cqn4sql(originalQuery, model) {
.findIndex(rl => rl.definition.isAssociation || rl.definition.kind === 'entity')
next = $refLinksReverse[nextStepIndex]
}
let as = getLastStringSegment(next.alias)
let as = getImplicitAlias(next.alias)
/**
* for an `expand` subquery, we do not need to add
* the table alias of the `expand` host to the join tree
* --> This is an artificial query, which will later be correlated
* with the main query alias. see @function expandColumn()
* There is one exception:
* - if current and next have the same alias, we need to assign a new alias to the next
*
*/
if (!(inferred.SELECT?.expand === true)) {
if (!(inferred.SELECT?.expand === true && current.alias.toLowerCase() !== as.toLowerCase())) {
as = getNextAvailableTableAlias(as)
}
next.alias = as
Expand Down Expand Up @@ -1920,9 +1928,11 @@ function cqn4sql(originalQuery, model) {
})
let pseudoPath = false
ref.reduce((prev, res, i) => {
if (res === '$self')
if (res === '$self') {
// next is resolvable in entity
thing.$refLinks.push({ definition: prev, target: prev })
return prev
}
if (res in pseudos.elements) {
pseudoPath = true
thing.$refLinks.push({ definition: pseudos.elements[res], target: pseudos })
Expand All @@ -1934,7 +1944,11 @@ function cqn4sql(originalQuery, model) {
const definition =
prev?.elements?.[res] || getDefinition(prev?.target)?.elements[res] || pseudos.elements[res]
const target = getParentEntity(definition)
thing.$refLinks[i] = { definition, target, alias: definition.name }
thing.$refLinks[i] = {
definition,
target,
alias: definition === assocRefLink.definition ? assocRefLink.alias : definition.name,
}
return prev?.elements?.[res] || getDefinition(prev?.target)?.elements[res] || pseudos.elements[res]
}, assocHost)
}
Expand All @@ -1959,15 +1973,16 @@ function cqn4sql(originalQuery, model) {
lhs.ref[0] in { $self: true, $projection: true } ? getParentEntity(assocRefLink.definition) : target,
)
else {
const lhsLeafArt = lhs.ref && lhs.$refLinks.at(-1).definition
const rhsLeafArt = rhs.ref && rhs.$refLinks.at(-1).definition
const lhsLeafDef = lhs.ref && lhs.$refLinks.at(-1).definition
const rhsLeafDef = rhs.ref && rhs.$refLinks.at(-1).definition
const lhsFirstDef = lhs.$refLinks.find(r => r.definition.kind === 'element')?.definition
// compare structures in on-condition
if ((lhsLeafArt?.target && rhsLeafArt?.target) || (lhsLeafArt?.elements && rhsLeafArt?.elements)) {
if ((lhsLeafDef?.target && rhsLeafDef?.target) || (lhsLeafDef?.elements && rhsLeafDef?.elements)) {
if (rhs.$refLinks[0].definition !== assocRefLink.definition) {
rhs.ref.unshift(targetSideRefLink.alias)
rhs.$refLinks.unshift(targetSideRefLink)
}
if (lhs.$refLinks[0].definition !== assocRefLink.definition) {
if (lhsFirstDef !== assocRefLink.definition) {
lhs.ref.unshift(targetSideRefLink.alias)
lhs.$refLinks.unshift(targetSideRefLink)
}
Expand All @@ -1977,16 +1992,15 @@ function cqn4sql(originalQuery, model) {
i += res.length
continue
}
// naive assumption: if first step is the association itself, all following ref steps must be resolvable
// assumption: if first step is the association itself, all following ref steps must be resolvable
// within target `assoc.assoc.fk` -> `assoc.assoc_fk`
else if (
lhs.$refLinks[0]?.definition ===
getParentEntity(assocRefLink.definition).elements[assocRefLink.definition.name]
lhsFirstDef === getParentEntity(assocRefLink.definition).elements[assocRefLink.definition.name]
)
result[i].ref = [assocRefLink.alias, lhs.ref.slice(1).join('_')]
result[i].ref = [assocRefLink.alias, lhs.ref.slice(lhs.ref[0] === '$self' ? 2 : 1).join('_')]
// naive assumption: if the path starts with an association which is not the association from
// which the on-condition originates, it must be a foreign key and hence resolvable in the source
else if (lhs.$refLinks[0]?.definition.target) result[i].ref = [result[i].ref.join('_')]
else if (lhsFirstDef?.target) result[i].ref = [result[i].ref.join('_')]
}
}
if (backlink) {
Expand All @@ -2009,7 +2023,7 @@ function cqn4sql(originalQuery, model) {
// 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`,
`Path step “${assocRefLink.definition.name}” is a self comparison with “${getFullName(backlink)}” that has no foreign keys`,
)
}
// managed backlink -> calculate fk-pk pairs
Expand Down Expand Up @@ -2307,7 +2321,7 @@ function cqn4sql(originalQuery, model) {
if (
inferred.SELECT?.from.uniqueSubqueryAlias &&
!inferred.SELECT?.from.as &&
firstStep === getLastStringSegment(transformedQuery.SELECT.from.ref[0])
getImplicitAlias(firstStep) === getImplicitAlias(transformedQuery.SELECT.from.ref[0])
) {
return inferred.SELECT?.from.uniqueSubqueryAlias
}
Expand All @@ -2316,7 +2330,7 @@ function cqn4sql(originalQuery, model) {
}

function getCombinedElementAlias(node) {
return getLastStringSegment(inferred.$combinedElements[node.ref[0].id || node.ref[0]]?.[0].index)
return inferred.$combinedElements[node.ref[0].id || node.ref[0]]?.[0].index
}
}
function getTransformedFunctionArgs(args, $baseLink = null) {
Expand Down Expand Up @@ -2389,17 +2403,6 @@ function hasLogicalOr(tokenStream) {
return tokenStream.some(t => t in { OR: true, or: true })
}

/**
* Returns the last segment of a string after the last dot.
*
* @param {string} str - The input string.
* @returns {string} The last segment of the string after the last dot. If there is no dot in the string, the function returns the original string.
*/
function getLastStringSegment(str) {
const index = str.lastIndexOf('.')
return index != -1 ? str.substring(index + 1) : str
}

function getParentEntity(element) {
if (element.kind === 'entity') return element
else return getParentEntity(element.parent)
Expand Down
12 changes: 6 additions & 6 deletions db-service/lib/infer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const cds = require('@sap/cds')

const JoinTree = require('./join-tree')
const { pseudos } = require('./pseudos')
const { isCalculatedOnRead } = require('../utils')
const { isCalculatedOnRead, getImplicitAlias } = require('../utils')
const cdsTypes = cds.linked({
definitions: {
Timestamp: { type: 'cds.Timestamp' },
Expand Down Expand Up @@ -92,7 +92,7 @@ function infer(originalQuery, model) {
* Each key is a query source alias, and its value is the corresponding CSN Definition.
* @returns {object} The updated `querySources` object with inferred sources from the `from` clause.
*/
function inferTarget(from, querySources) {
function inferTarget(from, querySources, useTechnicalAlias = true) {
const { ref } = from
if (ref) {
const { id, args } = ref[0]
Expand All @@ -114,14 +114,14 @@ function infer(originalQuery, model) {
from.uniqueSubqueryAlias ||
from.as ||
(ref.length === 1
? first.substring(first.lastIndexOf('.') + 1)
: (ref.at(-1).id || ref.at(-1)));
? getImplicitAlias(first, useTechnicalAlias)
: getImplicitAlias(ref.at(-1).id || ref.at(-1), useTechnicalAlias));
if (alias in querySources) throw new Error(`Duplicate alias "${alias}"`)
querySources[alias] = { definition: target, args }
const last = from.$refLinks.at(-1)
last.alias = alias
} else if (from.args) {
from.args.forEach(a => inferTarget(a, querySources))
from.args.forEach(a => inferTarget(a, querySources, false))
} else if (from.SELECT) {
const subqueryInFrom = infer(from, model) // we need the .elements in the sources
// if no explicit alias is provided, we make up one
Expand All @@ -131,7 +131,7 @@ function infer(originalQuery, model) {
} else if (typeof from === 'string') {
// TODO: Create unique alias, what about duplicates?
const definition = getDefinition(from) || cds.error`"${from}" not found in the definitions of your model`
querySources[from.substring(from.lastIndexOf('.') + 1)] = { definition }
querySources[getImplicitAlias(from, useTechnicalAlias)] = { definition }
} else if (from.SET) {
infer(from, model)
}
Expand Down
27 changes: 26 additions & 1 deletion db-service/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,34 @@ function isCalculatedElement(def) {
return def?.value
}

/**
* Calculates the implicit table alias for a given string.
*
* Based on the last part of the string, the implicit alias is calculated
* by taking the first character and prepending it with '$'.
* A leading '$' is removed if the last part already starts with '$'.
*
* @example
* getImplicitAlias('Books') => '$B'
* getImplicitAlias('bookshop.Books') => '$B'
* getImplicitAlias('bookshop.$B') => '$B'
*
* @param {string} str - The input string.
* @returns {string}
*/
function getImplicitAlias(str, useTechnicalAlias = true) {
const index = str.lastIndexOf('.')
if(useTechnicalAlias) {
const postfix = (index != -1 ? str.substring(index + 1) : str).replace(/^\$/, '')[0] || /* str === '$' */ '$'
return '$' + postfix
}
return index != -1 ? str.substring(index + 1) : str
}

// export the function to be used in other modules
module.exports = {
prettyPrintRef,
isCalculatedOnRead,
isCalculatedElement
isCalculatedElement,
getImplicitAlias,
}
Loading
Loading