fix: Improved placeholders and limit clause #567

BobdenOs · 2024-04-09T04:35:35Z

Added more cases that the SQL contains place holders. It now includes null and numbers. Now that it also includes numbers it is also used for the limit clause. With special behavior for HANA where the LIMIT is inline and the OFFSET is a place holder. Allowing the execution plan to create better optimizations.

… better-limit

johannes-vogel · 2024-04-11T07:08:16Z

db-service/test/cqn2sql/__snapshots__/delete.test.js.snap

@@ -1,8 +1,8 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

-exports[`delete test complex cascade delete for entity with 'not exists' 1`] = `"DELETE FROM Books as Books WHERE ( Books.author_id is not null or Books.author_version is not null ) and not exists (SELECT 1 as _exists FROM Author as Author left JOIN Books as parent ON parent.ID = Author.parent_ID WHERE Author.parent_ID = parent.code and parent.descr = Author.version)"`;
+exports[`delete test complex cascade delete for entity with 'not exists' 1`] = `"DELETE FROM Books as Books WHERE ( Books.author_id is not null or Books.author_version is not null ) and not exists (SELECT ? as _exists FROM Author as Author left JOIN Books as parent ON parent.ID = Author.parent_ID WHERE Author.parent_ID = parent.code and parent.descr = Author.version)"`;


Is there a way to detect SELECT 1? That's a common pattern in our implementation that we could also try to put in SQL string.

This is also a requirement for it to work with the hdb driver on HANA. As it checks the datatype of the place holder and of the data send to the placeholder. When doing SELECT ? the type of ? resolves to VARCHAR and then sending 1 the driver rejects the request. So now when a column is a {val:...} it will always be inlined as otherwise all type information is lost.

db-service/lib/cqn2sql.js

BobdenOs · 2024-04-11T09:35:50Z

Blocked: HDBCLIENT-1254 (planned: Q3.2024)

danjoa · 2024-04-19T19:11:56Z

db-service/lib/cqn2sql.js

@@ -290,7 +290,7 @@ class CQN2SQLRenderer {
 return `extensions__->${this.string('$."' + x.element.name + '"')} as ${x.as || x.element.name}`
 }
 ///////////////////////////////////////////////////////////////////////////////////////
- let sql = this.expr(x)
+ let sql = this.expr({ param: false, __proto__: x })


Why do we need that?

When selecting static values it is required to always inline them. Otherwise typed databases are confused what type it is supposed to return and causes trouble with SELECT 1 as SELECT ? will only accept strings as parameters as the default type of ? is nvarchar. Breaking a lot of queries.

danjoa · 2024-04-19T19:19:07Z

db-service/lib/cqn2sql.js

 * @param {import('./infer/cqn').val} param0
 * @returns {string} SQL
 */
 val({ val, param }) {
+ const inline = !this.values || param === false


Why did you pull that up here, if we still only need that down there in line 907, only if we reach that, of course?

danjoa · 2024-04-19T19:20:07Z

db-service/lib/cqn2sql.js

+ switch (typeof val) {
+ case 'string': return this.string(val)
+ case 'object': return 'NULL'
+ default:
+ return `${val}`
+ }


Why did the old code work before and not anymore?

Before the only value that would arrive here where strings. Now it can also be numbers and null. It could also have Booleans, but currently they are still being in lined.

danjoa · 2024-04-19T19:21:36Z

db-service/lib/cqn2sql.js

 case 'boolean': return `${val}`
- case 'number': return `${val}` // REVISIT for HANA


Why don't we pass numbers as bind parameters anymore? Has that advantages?

This PR is including numbers as binding parameters again.

…limit

… better-limit

🤖 I have created a release *beep* *boop* --- <details><summary>db-service: 1.9.0</summary> ## [1.9.0](db-service-v1.8.0...db-service-v1.9.0) (2024-05-08) ### Added * Add missing `func` cqn structures ([#629](#629)) ([9d7539a](9d7539a)) ### Fixed * **`order by`:** reject non-fk traversals of own columns in order by ([#599](#599)) ([3288d63](3288d63)) * Align all quote functions with @sap/cds-compiler ([#619](#619)) ([42e9828](42e9828)) * assign artificial alias if selecting from anonymous subquery ([#608](#608)) ([e1a7711](e1a7711)) * avoid spread operator ([#630](#630)) ([a39fb65](a39fb65)) * flat update with arbitrary where clauses ([#598](#598)) ([f108798](f108798)) * improved `=` and `!=` with val `null` for HANA and Postgres ([#626](#626)) ([cbcfe3b](cbcfe3b)) * Improved placeholders and limit clause ([#567](#567)) ([d5d5dbb](d5d5dbb)) * multiple result responses ([#602](#602)) ([bf0bed4](bf0bed4)) * only consider persisted columns for simple operations ([#592](#592)) ([6e31bda](6e31bda)) ### Changed * require `>= sap/cds-compiler@4.9` ([f4d09e2](f4d09e2)) * require `>= sap/cds@7.9.0` ([#627](#627)) ([f4d09e2](f4d09e2)) </details> <details><summary>sqlite: 1.7.0</summary> ## [1.7.0](sqlite-v1.6.0...sqlite-v1.7.0) (2024-05-08) ### Added * select decimals as strings if cds.env.features.string_decimals ([#616](#616)) ([39addbf](39addbf)) ### Fixed * Change `sql` property to `query` for errors ([#611](#611)) ([585577a](585577a)) * **hana:** Remove encoding from hana-client streams ([#623](#623)) ([fed8f6f](fed8f6f)) * Improved placeholders and limit clause ([#567](#567)) ([d5d5dbb](d5d5dbb)) </details> <details><summary>postgres: 1.8.0</summary> ## [1.8.0](postgres-v1.7.0...postgres-v1.8.0) (2024-05-08) ### Added * select decimals as strings if cds.env.features.string_decimals ([#616](#616)) ([39addbf](39addbf)) ### Fixed * Align all quote functions with @sap/cds-compiler ([#619](#619)) ([42e9828](42e9828)) * Change `sql` property to `query` for errors ([#611](#611)) ([585577a](585577a)) * Improved placeholders and limit clause ([#567](#567)) ([d5d5dbb](d5d5dbb)) * Use json datatype for Postgres insert ([#582](#582)) ([f1c9c89](f1c9c89)) </details> <details><summary>hana: 0.3.0</summary> ## [0.3.0](hana-v0.2.0...hana-v0.3.0) (2024-05-08) ### Added * select decimals as strings if cds.env.features.string_decimals ([#616](#616)) ([39addbf](39addbf)) ### Fixed * Add multi `concat` function to `@cap-js/hana` ([#624](#624)) ([df436fe](df436fe)) * Align all quote functions with @sap/cds-compiler ([#619](#619)) ([42e9828](42e9828)) * Change `sql` property to `query` for errors ([#611](#611)) ([585577a](585577a)) * Disconnect HANA tenant when deleted ([#589](#589)) ([a107db9](a107db9)) * **hana:** Align not found behavior in @cap-js/hana ([#603](#603)) ([54d2efb](54d2efb)) * **hana:** Allow custom fuzzy search cqn ([#620](#620)) ([80383f0](80383f0)) * **hana:** Allow HANA to use != and == inside xpr combinations ([#607](#607)) ([c578e9f](c578e9f)) * **hana:** Reference column alias in order by ([#615](#615)) ([7cd3a26](7cd3a26)) * **hana:** Remove encoding from hana-client streams ([#623](#623)) ([fed8f6f](fed8f6f)) * **hana:** Support associations with static values ([#604](#604)) ([05babcf](05babcf)) * improved `=` and `!=` with val `null` for HANA and Postgres ([#626](#626)) ([cbcfe3b](cbcfe3b)) * Improved placeholders and limit clause ([#567](#567)) ([d5d5dbb](d5d5dbb)) * Use json datatype for Postgres insert ([#582](#582)) ([f1c9c89](f1c9c89)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: Johannes Vogel <johannes.vogel@sap.com> Co-authored-by: Patrice Bender <patrice.bender@sap.com>

Improve place holders and limit clause

0b06653

BobdenOs requested review from danjoa, johannes-vogel, stewsk and patricebender as code owners April 9, 2024 04:35

BobdenOs and others added 4 commits April 10, 2024 14:53

Adjustments to handle type issues

17ccd57

Merge branch 'main' into better-limit

5268b41

fix count(1) for postgres

aba7b6b

Merge branch 'better-limit' of https://github.com/cap-js/cds-dbs into…

6a94493

… better-limit

BobdenOs requested review from gregorwolf, sebastianesch and vobu as code owners April 10, 2024 13:02

BobdenOs added 2 commits April 10, 2024 15:13

Update snapshot

b867ad0

Update db-service expectations

f0385de

johannes-vogel reviewed Apr 11, 2024

View reviewed changes

db-service/lib/cqn2sql.js Show resolved Hide resolved

BobdenOs added 2 commits April 11, 2024 11:54

Address type issues when using hdb driver

bde8a00

Allow count() on Postgres again

cc801d8

johannes-vogel approved these changes Apr 19, 2024

View reviewed changes

Merge branch 'main' into better-limit

129e7f1

danjoa reviewed Apr 19, 2024

View reviewed changes

BobdenOs added the next release pr to be checked for next release label Apr 24, 2024

BobdenOs and others added 7 commits April 24, 2024 14:59

Merge branch 'main' of https://github.com/cap-js/cds-dbs into better-…

70f7c24

…limit

cleanup left overs

f128fba

Merge branch 'better-limit' of https://github.com/cap-js/cds-dbs into…

2017c82

… better-limit

No longer transform != and == into is (not) null operator

01225d0

Allow null as placeholders in expression

c1a2175

Enforce data type for placeholders with coalesce

bfb1551

Merge branch 'main' into better-limit

d3a07c8

Convert = null to == null

81e0980

BobdenOs enabled auto-merge (squash) May 2, 2024 09:27

BobdenOs merged commit d5d5dbb into main May 2, 2024
4 of 6 checks passed

BobdenOs deleted the better-limit branch May 2, 2024 09:29

cap-bots mentioned this pull request May 2, 2024

chore: release main #587

Merged

This was referenced Jul 18, 2024

chore: release main #743

Merged

chore: release main #761

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Improved placeholders and limit clause #567

fix: Improved placeholders and limit clause #567

BobdenOs commented Apr 9, 2024

johannes-vogel Apr 11, 2024

BobdenOs Apr 11, 2024

BobdenOs commented Apr 11, 2024 •

edited

Loading

danjoa Apr 19, 2024

BobdenOs Apr 24, 2024

danjoa Apr 19, 2024

danjoa Apr 19, 2024

BobdenOs Apr 24, 2024

danjoa Apr 19, 2024

BobdenOs Apr 24, 2024

		case 'boolean': return `${val}`
		case 'number': return `${val}` // REVISIT for HANA

fix: Improved placeholders and limit clause #567

fix: Improved placeholders and limit clause #567

Conversation

BobdenOs commented Apr 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BobdenOs commented Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BobdenOs commented Apr 11, 2024 •

edited

Loading