From b3743a19b72b9bc44b7855578912c7482eade2ea Mon Sep 17 00:00:00 2001 From: Bob den Os <108393871+BobdenOs@users.noreply.github.com> Date: Mon, 16 Dec 2024 13:58:01 +0100 Subject: [PATCH] perf: Remove `$$RN$$` and additional sub selects (#929) 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 <31311694+johannes-vogel@users.noreply.github.com> --- hana/lib/HANAService.js | 84 ++++++++++++++++++++-------- test/compliance/SELECT.test.js | 28 +++++----- test/scenarios/bookshop/read.test.js | 6 +- 3 files changed, 79 insertions(+), 39 deletions(-) diff --git a/hana/lib/HANAService.js b/hana/lib/HANAService.js index a838eeade..681392e1e 100644 --- a/hana/lib/HANAService.js +++ b/hana/lib/HANAService.js @@ -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 } @@ -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))) { @@ -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: [ { @@ -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' @@ -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 } @@ -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 '*' @@ -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 @@ -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 } diff --git a/test/compliance/SELECT.test.js b/test/compliance/SELECT.test.js index 4d9bf04c7..786193446 100644 --- a/test/compliance/SELECT.test.js +++ b/test/compliance/SELECT.test.js @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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') }) diff --git a/test/scenarios/bookshop/read.test.js b/test/scenarios/bookshop/read.test.js index 622357b39..68506a4a3 100644 --- a/test/scenarios/bookshop/read.test.js +++ b/test/scenarios/bookshop/read.test.js @@ -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}])