Skip to content

Commit

Permalink
perf: Remove $$RN$$ and additional sub selects (#929)
Browse files Browse the repository at this point in the history 
When there is no need for `$$RN$$` it is excluded from the query. When
no additional internal columns are created the additional sub select is
collapsed into the primary select. Reducing the complexity of all
overall queries. This change mostly applies to queries using
`sql_simple_queries` as they don't require `$$RN$$` for `orderBy`
clauses.

---------

Co-authored-by: Johannes Vogel <[email protected]>
  • Loading branch information
@BobdenOs @johannes-vogel
BobdenOs and johannes-vogel authored Dec 16, 2024
1 parent a083f74 commit b3743a1
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 39 deletions.
84 changes: 62 additions & 22 deletions hana/lib/HANAService.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,18 @@ class HANAService extends SQLService {
const values = temporary
.map(t => {
const blobColumns = blobs.map(b => (b in t.blobs) ? blobColumn(b) : `NULL AS ${blobColumn(b)}`)
return `SELECT "_path_","_blobs_","_expands_","_json_"${blobColumns.length ? ',' : ''}${blobColumns} FROM (${t.select})`
return blobColumns.length
? `SELECT "_path_","_blobs_","_expands_","_json_",${blobColumns} FROM (${t.select})`
: t.select
})

const withclause = withclauses.length ? `WITH ${withclauses} ` : ''
const ret = withclause + (values.length === 1 ? values[0] : 'SELECT * FROM ' + values.map(v => `(${v})`).join(' UNION ALL ')) + ' ORDER BY "_path_" ASC'
const pathOrder = ' ORDER BY "_path_" ASC'
const ret = withclause + (
values.length === 1
? values[0] + (values[0].indexOf(`SELECT '$[' as "_path_"`) < 0 ? pathOrder : '')
: 'SELECT * FROM ' + values.map(v => `(${v})`).join(' UNION ALL ') + pathOrder
)
DEBUG?.(ret)
return ret
}
Expand Down Expand Up @@ -332,7 +339,7 @@ class HANAService extends SQLService {
throw new Error('CQN query using joins must specify the selected columns.')
}

let { limit, one, orderBy, expand, columns = ['*'], localized, count, parent } = q.SELECT
let { limit, one, from, orderBy, expand, columns = ['*'], localized, count, parent } = q.SELECT

// When one of these is defined wrap the query in a sub query
if (expand || (parent && (limit || one || orderBy))) {
Expand Down Expand Up @@ -385,23 +392,48 @@ class HANAService extends SQLService {
})
}

// Insert row number column for reducing or sorting the final result
const over = { xpr: [] }
// TODO: replace with full path partitioning
if (parent) over.xpr.push(`PARTITION BY ${this.ref({ ref: ['_parent_path_'] })}`)
if (orderBy?.length) over.xpr.push(` ORDER BY ${this.orderBy(orderBy, localized)}`)
const rn = { xpr: [{ func: 'ROW_NUMBER', args: [] }, 'OVER', over], as: '$$RN$$' }
q.as = q.SELECT.from.as
let hasBooleans = false
let hasExpands = false
let hasStructures = false
const aliasedOutputColumns = outputColumns.map(c => {
if (c.element?.type === 'cds.Boolean') hasBooleans = true
if (c.elements && c.element?.isAssociation) hasExpands = true
if (c.element?.type in this.BINARY_TYPES || c.elements || c.element?.elements || c.element?.items) hasStructures = true
return c.elements ? c : { __proto__: c, ref: [this.column_name(c)] }
})

const isSimpleQuery = (
cds.env.features.sql_simple_queries &&
(cds.env.features.sql_simple_queries > 1 || !hasBooleans) &&
!hasStructures &&
!parent
)

q = cds.ql.SELECT(['*', rn]).from(q)
q.as = q.SELECT.from.as
const rowNumberRequired = parent // If this query has a parent it is an expand
|| (!isSimpleQuery && (orderBy || from.SELECT)) // If using JSON functions the _path_ is used for top level sorting
|| hasExpands // Expands depend on parent $$RN$$

if (rowNumberRequired) {
// Insert row number column for reducing or sorting the final result
const over = { xpr: [] }
// TODO: replace with full path partitioning
if (parent) over.xpr.push(`PARTITION BY ${this.ref({ ref: ['_parent_path_'] })}`)
if (orderBy?.length) over.xpr.push(` ORDER BY ${this.orderBy(orderBy, localized)}`)
const rn = { xpr: [{ func: 'ROW_NUMBER', args: [] }, 'OVER', over], as: '$$RN$$' }
q.as = q.SELECT.from.as

q = cds.ql.SELECT(['*', rn]).from(q)
q.as = q.SELECT.from.as
}

q = cds.ql.SELECT(outputColumns.map(c => (c.elements ? c : { __proto__: c, ref: [this.column_name(c)] }))).from(q)
q.as = q.SELECT.from.as
Object.defineProperty(q, 'elements', { value: elements })
Object.defineProperty(q, 'element', { value: element })
if (rowNumberRequired || q.SELECT.columns.length !== aliasedOutputColumns.length) {
q = cds.ql.SELECT(aliasedOutputColumns).from(q)
q.as = q.SELECT.from.as
Object.defineProperty(q, 'elements', { value: elements })
Object.defineProperty(q, 'element', { value: element })
}

if (!q.SELECT.columns.find(c => c.as === '_path_')) {
if (rowNumberRequired && !q.SELECT.columns.find(c => c.as === '_path_')) {
q.SELECT.columns.push({
xpr: [
{
Expand Down Expand Up @@ -476,10 +508,11 @@ class HANAService extends SQLService {
const { SELECT, src } = q
if (!SELECT.columns) return '*'
const structures = []
const blobrefs = []
let expands = {}
let blobs = {}
let hasBooleans = false
let path = `'$['`
let path
let sql = SELECT.columns
.map(
SELECT.expand === 'root'
Expand Down Expand Up @@ -562,6 +595,7 @@ class HANAService extends SQLService {
return false
}
if (x.element?.type in this.BINARY_TYPES) {
blobrefs.push(x)
blobs[this.column_name(x)] = null
return false
}
Expand All @@ -577,7 +611,8 @@ class HANAService extends SQLService {
}
if (x.element?.type === 'cds.Boolean') hasBooleans = true
const converter = x.element?.[this.class._convertOutput] || (e => e)
return `${converter(this.quote(columnName), x.element)} as "${columnName.replace(/"/g, '""')}"`
const sql = x.param !== true && typeof x.val === 'number' ? this.expr({ param: false, __proto__: x }) : this.expr(x)
return `${converter(sql, x.element)} as "${columnName.replace(/"/g, '""')}"`
}
: x => {
if (x === '*') return '*'
Expand Down Expand Up @@ -609,7 +644,7 @@ class HANAService extends SQLService {
// Making each row a maximum size of 2gb instead of the whole result set to be 2gb
// Excluding binary columns as they are not supported by FOR JSON and themselves can be 2gb
const rawJsonColumn = sql.length
? `(SELECT ${sql} FROM JSON_TABLE('[{}]', '$' COLUMNS("'$$FaKeDuMmYCoLuMn$$'" FOR ORDINALITY)) FOR JSON ('format'='no', 'omitnull'='no', 'arraywrap'='no') RETURNS NVARCHAR(2147483647))`
? `(SELECT ${path ? sql : sql.map(c => c.slice(c.lastIndexOf(' as "') + 4))} FROM JSON_TABLE('{}', '$' COLUMNS("'$$FaKeDuMmYCoLuMn$$'" FOR ORDINALITY)) FOR JSON ('format'='no', 'omitnull'='no', 'arraywrap'='no') RETURNS NVARCHAR(2147483647))`
: `'{}'`

let jsonColumn = rawJsonColumn
Expand All @@ -629,11 +664,16 @@ class HANAService extends SQLService {

// Calculate final output columns once
let outputColumns = ''
outputColumns = `${this.quote('_path_')} as "_path_",${blobs} as "_blobs_",${expands} as "_expands_",${jsonColumn} as "_json_"`
outputColumns = `${path ? this.quote('_path_') : `'$['`} as "_path_",${blobs} as "_blobs_",${expands} as "_expands_",${jsonColumn} as "_json_"`
if (blobColumns.length)
outputColumns = `${outputColumns},${blobColumns.map(b => `${this.quote(b)} as "${b.replace(/"/g, '""')}"`)}`
this._outputColumns = outputColumns
sql = `*,${path} as ${this.quote('_path_')}`
if (path) {
sql = `*,${path} as ${this.quote('_path_')}`
} else {
structures.forEach(x => sql.push(this.column_expr(x)))
blobrefs.forEach(x => sql.push(this.column_expr(x)))
}
}
return sql
}
Expand Down
28 changes: 14 additions & 14 deletions test/compliance/SELECT.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,35 +471,35 @@ describe('SELECT', () => {

test('deep nested not', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not startswith(string,${'n'})`] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not startswith(string,${'n'})`] }} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('deep nested boolean function w/o operator', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`startswith(string,${'n'})`] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`startswith(string,${'n'})`] }} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'no')
})

test('deep nested not + and', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not startswith(string,${'n'}) and not startswith(string,${'n'})`] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not startswith(string,${'n'}) and not startswith(string,${'n'})`] }} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('multiple levels of not negations of expressions', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not startswith(string,${'n'})`] }] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not startswith(string,${'n'})`] }] }} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('multiple not in a single deep nested expression', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not not not startswith(string,${'n'})`] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [CXL`not not not startswith(string,${'n'})`] }} ORDER BY string DESC`
await cds.tx(async tx => {
let res
try {
Expand All @@ -514,14 +514,14 @@ describe('SELECT', () => {

test('multiple levels of not negations of expression with not + and', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not startswith(string,${'n'}) and not startswith(string,${'n'})`] }] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not startswith(string,${'n'}) and not startswith(string,${'n'})`] }] }} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('multiple levels of not negations of expression with multiple not in a single expression', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not not not startswith(string,${'n'}) and not not not startswith(string,${'n'})`] }] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', { xpr: ['not', CXL`not not not startswith(string,${'n'}) and not not not startswith(string,${'n'})`] }] }} ORDER BY string DESC`
await cds.tx(async tx => {
let res
try {
Expand All @@ -536,14 +536,14 @@ describe('SELECT', () => {

test('deep nested not before xpr with CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', CXL`string = 'no' ? true : false`] }] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', CXL`string = 'no' ? true : false`] }] }} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('deep nested multiple not before xpr with CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', 'not', 'not', CXL`string = 'no' ? true : false`] }] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', 'not', 'not', CXL`string = 'no' ? true : false`] }] }} ORDER BY string DESC`
await cds.tx(async tx => {
let res
try {
Expand All @@ -558,14 +558,14 @@ describe('SELECT', () => {

test('deep nested not before CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr] }] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr] }] }} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('deep nested multiple not before CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', 'not', 'not', ...(CXL`string = 'no' ? true : false`).xpr] }] }}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [{ xpr: ['not', 'not', 'not', ...(CXL`string = 'no' ? true : false`).xpr] }] }} ORDER BY string DESC`
await cds.tx(async tx => {
let res
try {
Expand All @@ -580,21 +580,21 @@ describe('SELECT', () => {

test('not before CASE statement', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr]}}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr]}} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})

test('and beetwen CASE statements', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [...(CXL`string = 'no' ? true : false`).xpr, 'and', ...(CXL`string = 'no' ? true : false`).xpr]}}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: [...(CXL`string = 'no' ? true : false`).xpr, 'and', ...(CXL`string = 'no' ? true : false`).xpr]}} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'no')
})

test('and beetwen CASE statements with not', async () => {
const { string } = cds.entities('basic.literals')
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr, 'and', 'not', ...(CXL`string = 'no' ? true : false`).xpr]}}`
const query = CQL`SELECT * FROM ${string} WHERE ${{ xpr: ['not', ...(CXL`string = 'no' ? true : false`).xpr, 'and', 'not', ...(CXL`string = 'no' ? true : false`).xpr]}} ORDER BY string DESC`
const res = await cds.run(query)
assert.strictEqual(res[0].string, 'yes')
})
Expand Down
6 changes: 3 additions & 3 deletions test/scenarios/bookshop/read.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,9 @@ describe('Bookshop - Read', () => {

it('allows various mechanisms for expressing "not in"', async () => {
const results = await cds.db.run([
SELECT.from('sap.capire.bookshop.Books', ['ID']).where({ ID: { 'not in': [201, 251] } }),
SELECT.from('sap.capire.bookshop.Books', ['ID']).where({ ID: { not: { in: [201, 251] } } }),
SELECT.from('sap.capire.bookshop.Books', ['ID']).where('ID not in', [201, 251])
SELECT.from('sap.capire.bookshop.Books', ['ID']).where({ ID: { 'not in': [201, 251] } }).orderBy('ID'),
SELECT.from('sap.capire.bookshop.Books', ['ID']).where({ ID: { not: { in: [201, 251] } } }).orderBy('ID'),
SELECT.from('sap.capire.bookshop.Books', ['ID']).where('ID not in', [201, 251]).orderBy('ID'),
])

for (const row of results) expect(row).to.deep.eq([{ID: 207},{ID: 252},{ID: 271}])
Expand Down

0 comments on commit b3743a1

Please sign in to comment.