-
Notifications
You must be signed in to change notification settings - Fork 14
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: simple queries #654
feat: simple queries #654
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
would it make sense to have a similar option for the other db's as well? Sometimes, for debugging purposes, I wished to have the possibility to get rid of the json
noise. If we had this for all dbs, we might find a more generic option name.
This is for customers which believe these queries will be faster for their use cases. As the other databases are very fast in rendering json. There is no reason to do this. Also the output result is unstable. |
🤖 I have created a release *beep* *boop* --- <details><summary>db-service: 1.10.0</summary> ## [1.10.0](db-service-v1.9.2...db-service-v1.10.0) (2024-05-29) ### Added * Add simple queries feature flag ([#660](#660)) ([3335202](3335202)) </details> <details><summary>postgres: 1.9.0</summary> ## [1.9.0](postgres-v1.8.0...postgres-v1.9.0) (2024-05-29) ### Added * Add simple queries feature flag ([#660](#660)) ([3335202](3335202)) </details> <details><summary>hana: 0.5.0</summary> ## [0.5.0](hana-v0.4.0...hana-v0.5.0) (2024-05-29) ### Added * simple queries ([#654](#654)) ([ba77f9e](ba77f9e)) </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: sjvans <30337871+sjvans@users.noreply.github.com>
@BobdenOs How is the case handled, where a query has an How is it now with your changes of this PR? We liked the approach, that only a single placeholder is used because this would make the execution plan stable, no matter how many entries are in the array (which is often not under our control, as depending on user data)... Having some toggles, that would provide different version would be good, so that we can experiment with that. What do you think? FYI @D069515 |
Adds
hana_simple_queries
feature flag. Which will make@cap-js/hana
run all possible queries withoutFOR JSON
. Which allowshdb
and@sap/hana-client
to returns different results again.Output converters are kept in place to reduce the likely hood of this happening. Current impacted test is
Bookshop - Order By › collations for aggregating queries with subselect
. Where the result ofsum(price) as pri
comes back as"150"
for@sap/hana-client
and"1.5e+2"
forhdb
. While expected is150
in theFOR JSON
result.Additionally this PR removes the duplicate
"_json_"
output definition. Simplifying the SQL in all cases.