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: Simplifies count queries #705

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 5 additions & 17 deletions db-service/lib/SQLService.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ class SQLService extends DatabaseService {
if (!query.target) {
try { this.infer(query) } catch { /**/ }
}
if (query.target && !query.target._unresolved) {
if (query.SELECT.expand !== false && query.target && !query.target._unresolved) {
// Will return multiple rows with objects inside
query.SELECT.expand = 'root'
}

const { sql, values, cqn } = this.cqn2sql(query, data)
const expand = query.SELECT.expand
delete query.SELECT.expand
if (expand === 'root') delete query.SELECT.expand

let ps = await this.prepare(sql)
let rows = await ps.all(values)
Expand Down Expand Up @@ -292,21 +292,9 @@ class SQLService extends DatabaseService {
if (max === undefined || (n < max && (n || !offset))) return n + offset
}

// Keep original query columns when potentially used insde conditions
const { having, groupBy } = query.SELECT
const columns = (having?.length || groupBy?.length)
? query.SELECT.columns.filter(c => !c.expand)
: [{ val: 1 }]
const cq = SELECT.one([{ func: 'count' }]).from(
cds.ql.clone(query, {
columns,
localized: false,
expand: false,
limit: undefined,
orderBy: undefined,
}),
)
const { count } = await this.onSELECT({ query: cq })
const cq = this.class.CQN2SQL.count(query)
cq.SELECT.expand = false // Force count queries to be simple
const [{ count }] = await this.onSELECT({ query: cq })
return count
}

Expand Down
67 changes: 48 additions & 19 deletions db-service/lib/cqn2sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,31 +269,36 @@ class CQN2SQLRenderer {
q.elements[e].items // Array types require to be inlined with a json result
)

let hasConverters = false
let cols = SELECT.columns.map(isSimple
? x => {
const name = this.column_name(x)
const escaped = `${name.replace(/"/g, '""')}`
let col = `${this.output_converter4(x.element, this.quote(name))} AS "${escaped}"`
const converter = x.element?.[this.class._convertOutput]
hasConverters = hasConverters || !!converter
const quoted = this.quote(name)
let col = converter?.(quoted, x.element) || quoted
if (x.SELECT?.count) {
hasConverters = true
// REVISIT: should be moved to protocol adapters
// Return both the sub select and the count for @odata.count
const qc = cds.ql.clone(x, { columns: [{ func: 'count' }], one: 1, limit: 0, orderBy: 0 })
return [col, `${this.expr(qc)} AS "${escaped}@odata.count"`]
return [col, `${this.count(x)} AS "${name.replace(/"/g, '""')}@odata.count"`]
}
return col
}
: x => {
const name = this.column_name(x)
const escaped = `${name.replace(/"/g, '""')}`
let col = `'$."${escaped}"',${this.output_converter4(x.element, this.quote(name))}`
const converter = x.element?.[this.class._convertOutput] || (a => a)
let col = `'$."${escaped}"',${converter(this.quote(name), x.element)}`
if (x.SELECT?.count) {
// REVISIT: should be moved to protocol adapters
// Return both the sub select and the count for @odata.count
const qc = cds.ql.clone(x, { columns: [{ func: 'count' }], one: 1, limit: 0, orderBy: 0 })
return [col, `'$."${escaped}@odata.count"',${this.expr(qc)}`]
return [col, `'$."${escaped}@odata.count"',${this.count(x)}`]
}
return col
}).flat()

if (isSimple) return `SELECT ${cols} FROM (${sql})`
if (isSimple) return hasConverters ? `SELECT ${cols} FROM (${sql})` : sql

// Prevent SQLite from hitting function argument limit of 100
let obj = "'{}'"
Expand Down Expand Up @@ -702,17 +707,6 @@ class CQN2SQLRenderer {
return this.sql
}

/**
* Wraps the provided SQL expression for output processing
* @param {import('./infer/cqn').element} element
* @param {string} expr
* @returns {string} SQL
*/
output_converter4(element, expr) {
const fn = element?.[this.class._convertOutput]
return fn?.(expr, element) || expr
}

/** @type {import('./converters').Converters} */
static InputConverters = {} // subclasses to override

Expand Down Expand Up @@ -1042,6 +1036,41 @@ class CQN2SQLRenderer {
return s
}

/**
* Converts the provided query into the correct count query
* @param {import('./infer/cqn').SELECT} q
* @returns {import('./infer/cqn').SELECT} count query
*/
static count(q) {
const { where, having, groupBy } = q.SELECT
// Keep original query columns when potentially used insde conditions
const hasAfterConditions = (having?.length || groupBy?.length)
const hasConditions = hasAfterConditions || where?.length
// Don't use SELECT.one as it adds an limit 1 to the query which is not needed
return SELECT([{ func: 'count' }]).from(
q.target && !hasConditions
? q.target // Just count from the entity directly
: cds.ql.clone(q, {
columns: hasAfterConditions
? q.SELECT.columns.filter(c => !c.expand)
: [{ val: 1 }],
localized: false,
expand: undefined,
limit: undefined,
orderBy: undefined,
}),
)
}

/**
* Converts the provided query into the correct count query
* @param {import('./infer/cqn').SELECT} q
* @returns {string} SQL
*/
count(q) {
return this.expr(this.class.count(q))
}

/**
* Convers the columns array into an array of SQL expressions that extract the correct value from inserted JSON data
* @param {object[]} columns
Expand Down
24 changes: 15 additions & 9 deletions hana/lib/HANAService.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ class HANAService extends SQLService {
}

const isLockQuery = query.SELECT.forUpdate || query.SELECT.forShareLock
if (!isLockQuery) {
// REVISIT: disable this for queries like (SELECT 1)
if (!isLockQuery && query.SELECT.expand !== false) {
// Will return multiple rows with objects inside
query.SELECT.expand = 'root'
}
Expand Down Expand Up @@ -349,7 +348,7 @@ class HANAService extends SQLService {
q.SELECT.one = undefined
q.SELECT.orderBy = undefined
}
q.SELECT.expand = false
q.SELECT.expand = undefined

const outputColumns = [...columns.filter(c => c.as !== '_path_')]

Expand Down Expand Up @@ -521,12 +520,19 @@ class HANAService extends SQLService {
const converter = x.element?.[this.class._convertOutput] || (e => e)
return `${converter(this.quote(columnName))} as "${columnName.replace(/"/g, '""')}"`
}
: x => {
if (x === '*') return '*'
// means x is a sub select expand
if (x.elements) return false
return this.column_expr(x)
},
: SELECT.expand === false // Force simple query
? x => {
if (x === '*') return '*'
// means x is a sub select expand
if (x.elements) return false
return `${this.column_expr({ __proto__: x, as: '' })} AS "${this.column_name(x).replace(/"/g, '""')}"`
}
: x => {
if (x === '*') return '*'
// means x is a sub select expand
if (x.elements) return false
return this.column_expr(x)
},
)
.filter(a => a)

Expand Down
8 changes: 4 additions & 4 deletions postgres/lib/PostgresService.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,13 +376,13 @@ GROUP BY k
const queryAlias = this.quote(SELECT.from?.as || (SELECT.expand === 'root' && 'root'))
const cols = SELECT.columns.map(x => {
const name = this.column_name(x)
const outputConverter = this.output_converter4(x.element, `${queryAlias}.${this.quote(name)}`)
let col = `${outputConverter} as ${this.doubleQuote(name)}`
const outputConverter = x.element?.[this.class._convertOutput] || (a => a)
let col = `${outputConverter(`${queryAlias}.${this.quote(name)}`, x.element)} as ${this.doubleQuote(name)}`

if (x.SELECT?.count) {
// REVISIT: should be moved to protocol adapters
// Return both the sub select and the count for @odata.count
const qc = cds.ql.clone(x, { columns: [{ func: 'count' }], one: 1, limit: 0, orderBy: 0 })
col += `,${this.expr(qc)} as ${this.doubleQuote(`${name}@odata.count`)}`
col += `,${this.count(x)} as ${this.doubleQuote(`${name}@odata.count`)}`
}
return col
})
Expand Down
Loading