cap-js / cds-dbs

Monorepo for SQL Database Services for CAP
https://cap.cloud.sap/docs/
Apache License 2.0
37 stars 11 forks source link

`sql_simple_queries` + `odata_new_adapter` breaks `ieee754compatible` setting #838

Closed hakimio closed 1 month ago

hakimio commented 1 month ago

Description of erroneous behaviour

Combination of sql_simple_queries and odata_new_adapter features breaks "ieee754compatible": false setting.

Detailed steps to reproduce

Details about your project

Package Version
@cap-js/asyncapi 1.0.2
@cap-js/cds-types 0.6.5
@cap-js/openapi 1.0.6
@cap-js/postgres 1.10.0
@sap/cds 8.3.1
@sap/cds-compiler 5.3.2
@sap/cds-dk 8.3.0
@sap/cds-foss 5.0.1
@sap/cds-mtxs 2.2.0
@sap/eslint-plugin-cds 3.1.0
Node.js v20.13.1
BobdenOs commented 1 month ago

@hakimio when ieee754compatible is disabled @cap-js/sqlite and @cap-js/postgres won't apply the IEEE754 output converters and therefor the results won't be IEEE754 compliant. This should be the case as long as you use odata_new_adapter as the old adapter would post process all the results. When odata_new_adapter is set it automatically turns on ieee754compatible to make sure that the results match up, but if you actively disable the feature flag it should output non compliant results.

hakimio commented 1 month ago

@BobdenOs ieee754compatible setting works correctly with odata_new_adapter as long as sql_simple_queries is not enabled. The issue is with sql_simple_queries not odata_new_adapter.

hakimio commented 1 month ago

Test cases: ✔ odata_new_adapter: false + sql_simple_queries: 1 + ieee754compatible: falseodata_new_adapter: true + sql_simple_queries: 0 + ieee754compatible: falseodata_new_adapter: true + sql_simple_queries: 1 + ieee754compatible: false

The last case inconsistently outputs numbers as strings.

hakimio commented 1 month ago

@BobdenOs

if you actively disable the feature flag it should output non compliant results

It doesn't as explained above.