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

feat(search): enable deep search with path expressions #590

Merged
merged 43 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
a194978
feat: deep search
patricebender Apr 16, 2024
6cd256e
comments
patricebender Apr 16, 2024
38652cb
add some end 2 end tests, remove caching
patricebender Apr 17, 2024
5a6f1ea
remove unused import
patricebender Apr 17, 2024
84963a0
skip calculated element tests for now
patricebender Apr 17, 2024
d2ae197
add test which adds new searchable fields dynamically
patricebender Apr 17, 2024
7d4ab5f
cosmetics
patricebender Apr 17, 2024
34b0ee3
Merge remote-tracking branch 'origin/main' into patrice/search
patricebender Apr 17, 2024
ab150ed
use inferred target for search resolving
patricebender Apr 22, 2024
82e73fa
Merge branch 'main' into patrice/search
patricebender Apr 22, 2024
11747f2
Merge branch 'main' into patrice/search
patricebender Apr 23, 2024
2521ff6
unskip tests, add todo
patricebender Apr 23, 2024
6982cea
Merge remote-tracking branch 'origin/main' into patrice/search
patricebender Apr 25, 2024
e6927b0
add search cache
patricebender Apr 25, 2024
f97e880
improve end2end tests
patricebender Apr 25, 2024
f667624
re-enable caching test
patricebender Apr 25, 2024
8cb5962
port everything to cqn4sql
patricebender Apr 27, 2024
7471bdb
minor adjustements
patricebender Apr 29, 2024
d2095e8
Merge branch 'main' into patrice/search
patricebender May 6, 2024
bce5054
we need cds@7.9.0
patricebender May 6, 2024
3a1c52f
Revert "we need cds@7.9.0"
patricebender May 7, 2024
e9eeed0
Merge remote-tracking branch 'origin/main' into patrice/search
patricebender May 7, 2024
bba5502
Merge branch 'main' into patrice/search
patricebender May 10, 2024
578cd0b
Merge branch 'main' into patrice/search
patricebender May 13, 2024
156c664
Merge branch 'main' into patrice/search
johannes-vogel May 13, 2024
9f8fd58
minor improvements
patricebender May 14, 2024
87b5f63
remove accidental change
patricebender May 14, 2024
5eb8ca0
return custom joins as is
patricebender May 14, 2024
e88113b
Merge branch 'main' into patrice/search
johannes-vogel Jun 7, 2024
4b3b7ef
infer custom join queries
patricebender Jun 7, 2024
fd559da
safeguard
patricebender Jun 7, 2024
53dbd1f
run all tests
patricebender Jun 7, 2024
98f3662
add another workaround
patricebender Jun 7, 2024
0ae8826
Merge branch 'main' into patrice/search
patricebender Jun 14, 2024
5809f04
Merge branch 'main' into patrice/search
johannes-vogel Jun 17, 2024
59578f6
expand subquery must be type of ql.Query
patricebender Jun 18, 2024
9f09795
Merge branch 'main' into patrice/search
patricebender Jun 18, 2024
d5d30e5
dont rely on cds.infer for expand subqueries
patricebender Jun 19, 2024
3cf4598
Merge branch 'main' into patrice/search
patricebender Jun 19, 2024
48ccb18
Merge branch 'main' into patrice/search
johannes-vogel Jul 5, 2024
6337006
Merge branch 'main' into patrice/search
johannes-vogel Jul 5, 2024
8b7ac43
apply suggestions
patricebender Jul 5, 2024
4dbcf83
move comment
patricebender Jul 5, 2024
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
50 changes: 19 additions & 31 deletions db-service/lib/cqn4sql.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict'

const cds = require('@sap/cds/lib')
const { computeColumnsToBeSearched } = require('./search')

const infer = require('./infer')

Expand Down Expand Up @@ -179,9 +178,9 @@ function cqn4sql(originalQuery, model) {
}
}

if (inferred.SELECT.search) {
if (inferred.SELECT.search?.searchTerm) {
// Search target can be a navigation, in that case use _target to get the correct entity
const { where, having } = transformSearch(inferred.SELECT.search, transformedFrom) || {}
const { where, having } = transformSearch(inferred.SELECT.search)
if (where) transformedQuery.SELECT.where = where
else if (having) transformedQuery.SELECT.having = having
}
Expand Down Expand Up @@ -209,43 +208,32 @@ function cqn4sql(originalQuery, model) {
* @param {object} search - The search expression to be applied to the searchable columns within the query source.
* @param {object} from - The FROM clause of the CQN statement.
*
* @returns {(Object|Array|null)} - The function returns an object representing the WHERE or HAVING clause of the query:
* - If the target of the query contains searchable elements, an array representing the WHERE or HAVING clause is returned.
* This includes appending to an existing clause with an AND condition or creating a new clause solely with the 'contains' clause.
* - If the SELECT query does not initially contain a WHERE or HAVING clause, the returned object solely consists of the 'contains' clause.
* - If the target entity of the query does not contain searchable elements, the function returns null.
* @returns {Object} - The function returns an object representing the WHERE or HAVING clause of the query.
*
* Note: The WHERE clause is used for filtering individual rows before any aggregation occurs.
* The HAVING clause is utilized for conditions on aggregated data, applied after grouping operations.
*/
function transformSearch(search, from) {
const entity = getDefinition(from.$refLinks[0].definition.target) || from.$refLinks[0].definition
function transformSearch(search) {
const searchTokenStream = [search.searchTerm]
const searchTerm = getTransformedTokenStream(searchTokenStream)[0]
// pass transformedQuery because we may need to search in the columns directly
// in case of aggregation
const searchIn = computeColumnsToBeSearched(transformedQuery, entity, from.as)
if (searchIn.length > 0) {
const xpr = search
const contains = {
func: 'search',
args: [
searchIn.length > 1 ? { list: searchIn } : { ...searchIn[0] },
xpr.length === 1 && 'val' in xpr[0] ? xpr[0] : { xpr },
],
}
const contains = searchTerm
patricebender marked this conversation as resolved.
Show resolved Hide resolved

// if the query is grouped and the queries columns contain an aggregate function,
// we must put the search term into the `having` clause, as the search expression
// is defined on the aggregated result, not on the individual rows
let prop = 'where'
// if the query is grouped and the queries columns contain an aggregate function,
// we must put the search term into the `having` clause, as the search expression
// is defined on the aggregated result, not on the individual rows
let prop = 'where'

if (inferred.SELECT.groupBy && searchIn.some(c => c.func || c.xpr)) prop = 'having'
if (transformedQuery.SELECT[prop]) {
return { [prop]: [asXpr(transformedQuery.SELECT.where), 'and', contains] }
} else {
return { [prop]: [contains] }
}
const usesAggregation =
inferred.SELECT.groupBy &&
(searchTerm.args[0].func || searchTerm.args[0].xpr || searchTerm.args[0].list?.some(c => c.func || c.xpr))

if (usesAggregation) prop = 'having'
if (transformedQuery.SELECT[prop]) {
return { [prop]: [asXpr(transformedQuery.SELECT.where), 'and', contains] }
} else {
return null
return { [prop]: [contains] }
}
}

Expand Down
53 changes: 48 additions & 5 deletions db-service/lib/infer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const cds = require('@sap/cds/lib')

const { computeColumnsToBeSearched } = require('../search')
const JoinTree = require('./join-tree')
const { pseudos } = require('./pseudos')
const cdsTypes = cds.linked({
Expand Down Expand Up @@ -286,7 +287,7 @@ function infer(originalQuery, model) {
*/
function inferQueryElements($combinedElements) {
let queryElements = {}
const { columns, where, groupBy, having, orderBy } = _
const { columns, where, groupBy, having, orderBy, search } = _
if (!columns) {
inferElementsFromWildCard(aliases)
} else {
Expand Down Expand Up @@ -355,6 +356,16 @@ function infer(originalQuery, model) {
if (_.with)
// consider UPDATE.with
Object.values(_.with).forEach(val => inferQueryElement(val, false))
if (search) {
patricebender marked this conversation as resolved.
Show resolved Hide resolved
const searchTerm = getSearchTerm(inferred.SELECT.search, inferred.SELECT.from)
if (searchTerm) {
searchTerm.args.forEach(arg => inferQueryElement(arg, false))
Object.defineProperty(search, 'searchTerm', {
writable: true,
value: searchTerm,
})
}
}

return queryElements

Expand Down Expand Up @@ -727,8 +738,10 @@ function infer(originalQuery, model) {
function resolveInline(col, namePrefix = col.as || col.flatName) {
const { inline, $refLinks } = col
const $leafLink = $refLinks[$refLinks.length - 1]
if(!$leafLink.definition.target && !$leafLink.definition.elements) {
throw new Error(`Unexpected “inline” on “${col.ref.map(idOnly)}”; can only be used after a reference to a structure, association or table alias`)
if (!$leafLink.definition.target && !$leafLink.definition.elements) {
throw new Error(
`Unexpected “inline” on “${col.ref.map(idOnly)}”; can only be used after a reference to a structure, association or table alias`,
)
}
let elements = {}
inline.forEach(inlineCol => {
Expand Down Expand Up @@ -783,8 +796,10 @@ function infer(originalQuery, model) {
function resolveExpand(col) {
const { expand, $refLinks } = col
const $leafLink = $refLinks?.[$refLinks.length - 1] || inferred.SELECT.from.$refLinks.at(-1) // fallback to anonymous expand
if(!$leafLink.definition.target && !$leafLink.definition.elements) {
throw new Error(`Unexpected “expand” on “${col.ref.map(idOnly)}”; can only be used after a reference to a structure, association or table alias`)
if (!$leafLink.definition.target && !$leafLink.definition.elements) {
throw new Error(
`Unexpected “expand” on “${col.ref.map(idOnly)}”; can only be used after a reference to a structure, association or table alias`,
)
}
const target = getDefinition($leafLink.definition.target)
if (target) {
Expand Down Expand Up @@ -1154,6 +1169,34 @@ function infer(originalQuery, model) {
return res !== '' ? res + dot + cur.definition.name : cur.definition.name
}, '')
}
/**
* For a given search expression return a function "search" which holds the search expression
* as well as the searchable columns as arguments.
*
* @param {object} search - The search expression which shall be applied to the searchable columns on the query source.
* @param {object} from - The FROM clause of the CQN statement.
*
* @returns {(Object|null)} returns either:
* - a function with two arguments: The first one being the list of searchable columns, the second argument holds the search expression.
* - or null, if no searchable columns are found in neither in `@cds.search` nor in the target entity itself.
*/
function getSearchTerm(search, from) {
const entity = from.$refLinks.at(-1).definition._target || from.$refLinks.at(-1).definition
const searchIn = computeColumnsToBeSearched(inferred, entity, from.as)
if (searchIn.length > 0) {
const xpr = search
const contains = {
func: 'search',
args: [
searchIn.length > 1 ? { list: searchIn } : { ...searchIn[0] },
xpr.length === 1 && 'val' in xpr[0] ? xpr[0] : { xpr },
],
}
return contains
} else {
return null
}
}
}

/**
Expand Down
53 changes: 34 additions & 19 deletions db-service/lib/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

for (const each in elements) {
const element = elements[each]
if (element.isAssociation) continue
if (filterVirtual && element.virtual) continue
if (removeIgnore && element['@cds.api.ignore']) continue
if (filterDraft && each in DRAFT_COLUMNS_UNION) continue
Expand Down Expand Up @@ -68,20 +67,20 @@
}

let atLeastOneColumnIsSearchable = false
const deepSearchCandidates = []

// build a map of columns annotated with the @cds.search annotation
for (const key of cdsSearchKeys) {
const columnName = key.split(cdsSearchTerm + '.').pop()

// REVISIT: for now, exclude search using path expression, as deep search is not currently
// supported
if (columnName.includes('.')) {
continue
}

const annotationKey = `${cdsSearchTerm}.${columnName}`
const annotationValue = entity[annotationKey]
if (annotationValue) atLeastOneColumnIsSearchable = true

const column = entity.elements[columnName]
if (column?.isAssociation || columnName.includes('.')) {
deepSearchCandidates.push({ ref: columnName.split('.') })
continue;
}
cdsSearchColumnMap.set(columnName, annotationValue)
}

Expand All @@ -92,6 +91,9 @@
// `@cds.search { element1: true }` or `@cds.search { element1 }`
if (annotatedColumnValue) return true

// calculated elements are only searchable if requested through `@cds.search`
if(column.value) return false

// if at least one element is explicitly annotated as searchable, e.g.:
// `@cds.search { element1: true }` or `@cds.search { element1 }`
// and it is not the current column name, then it must be excluded from the search
Expand All @@ -106,21 +108,35 @@
)
})

// if the @cds.search annotation is provided -->
// Early return to ignore the interpretation of the @Search.defaultSearchElement
// annotation when an entity is annotated with the @cds.search annotation.
// The @cds.search annotation overrules the @Search.defaultSearchElement annotation.
if (cdsSearchKeys.length > 0) {
return searchableColumns.map(column => column.name)
if (deepSearchCandidates.length) {
deepSearchCandidates.forEach(c => {
const element = c.ref.reduce((resolveIn, curr, i) => {
const next = resolveIn.elements?.[curr] || resolveIn._target.elements[curr]
if (next.isAssociation && !c.ref[i + 1]) {
const searchInTarget = _getSearchableColumns(next._target)
searchInTarget.forEach(elementRefInTarget => {
searchableColumns.push({ ref: c.ref.concat(...elementRefInTarget.ref) })
})
}
return next
}, entity)
if (element?.type === DEFAULT_SEARCHABLE_TYPE) {
searchableColumns.push({ ref: c.ref })
}
})
}

return searchableColumns.map(column => column.name)
return searchableColumns.map(column => {
if(column.ref)
return column
return { ref: [ column.name ] }
})
}

/**
* @returns {Array<object>} - array of columns
*/
const computeColumnsToBeSearched = (cqn, entity = { __searchableColumns: [] }, alias) => {

Check warning on line 139 in db-service/lib/search.js

View workflow job for this annotation

GitHub Actions / Node.js 18

'alias' is defined but never used. Allowed unused args must match /lazy/u

Check warning on line 139 in db-service/lib/search.js

View workflow job for this annotation

GitHub Actions / HANA Node.js 18

'alias' is defined but never used. Allowed unused args must match /lazy/u
let toBeSearched = []

// aggregations case
Expand Down Expand Up @@ -163,11 +179,10 @@
}
})
} else {
toBeSearched = entity.own('__searchableColumns') || entity.set('__searchableColumns', _getSearchableColumns(entity))
if (cqn.SELECT.groupBy) toBeSearched = toBeSearched.filter(tbs => cqn.SELECT.groupBy.some(gb => gb.ref[0] === tbs))
// no caching, as it makes it impossible to dynamically assign/unassign the @cds.search annotation multiple times
toBeSearched = _getSearchableColumns(entity)
toBeSearched = toBeSearched.map(c => {
const column = { ref: [c] }
if (alias) column.ref.unshift(alias)
const column = {ref: [...c.ref]}
return column
})
}
Expand Down
72 changes: 72 additions & 0 deletions db-service/test/bookshop/db/search.cds
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
namespace search;

entity Books {
key ID : Integer;
title : String;
author : Association to Authors;
coAuthor_ID_unmanaged : Integer;
coAuthorUnmanaged : Association to Authors
on coAuthorUnmanaged.ID = coAuthor_ID_unmanaged;
}

@cds.search: {author.lastName}
entity BooksSearchAuthorName : Books {}

// search through all searchable fields in the author
@cds.search: {author}
entity BooksSearchAuthor : Books {}

entity Authors {
key ID : Integer;
lastName : String;
firstName : String;
books : Association to Books
on books.author = $self;
}

// search over multiple associations
@cds.search: {authorWithAddress}
entity BooksSearchAuthorAndAddress : Books {
authorWithAddress : Association to AuthorsSearchAddresses;
}

@cds.search: {
address,
note
}
entity AuthorsSearchAddresses : Authors {
note : String;
address : Association to Addresses;
}

@cds.search: {street: false}
entity Addresses {
key ID : Integer;
street : String;
city : String;
zip : Integer;
}

// search with calculated elements

@cds.search: {
address,
note
}
entity AuthorsSearchCalculatedAddress : Authors {
note : String;
address : Association to CalculatedAddresses;
}

@cds.search: {
city : false,
calculatedAddress: true
}
entity CalculatedAddresses : Addresses {
calculatedAddress : String = street || ' ' || zip || '' || city
}

// calculated elements are not searchable by default
entity CalculatedAddressesWithoutAnno : Addresses {
calculatedAddress : String = street || ' ' || zip || '' || city
}
Loading
Loading