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

@cap-js/hana - Save of draft entity (with active entity) containing ST_GEOMETRY property returns error "Wrong input for BINARY type" #824

Closed mathew-rizing closed 2 weeks ago

mathew-rizing commented 1 month ago

Description of erroneous behaviour

Updating entity properties of type ST_GEOMETRY like so:

await UPDATE(req.subject).with({geometry: "SRID=4326;POINT (1.62965918650832 3.1377292657822693)"});

Results in the following error:

[sql] - BEGIN
[sql] - UPDATE demo_Headers AS Headers SET geometry=? WHERE Headers.ID = ? SRID=4326;POINT (1.62965918650832 3.1377292657822693) 2d4262ad-5ad9-4d9f-bcd4-180ff1e8b40e
[cds] - Error: Cannot set parameter at row: 1. Wrong input for BINARY type
    at next (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\protocol\ExecuteTask.js:175:19)
    at ExecuteTask.getParameters (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\protocol\ExecuteTask.js:195:3)
    at ExecuteTask.sendExecute (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\protocol\ExecuteTask.js:202:8)
    at execute (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\protocol\ExecuteTask.js:73:10)
    at ExecuteTask.run (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\protocol\ExecuteTask.js:115:3)
    at run (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\util\Queue.js:97:12)
    at Queue.dequeue (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\util\Queue.js:100:3)
    at Queue.push (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\util\Queue.js:48:10)
    at Connection.enqueue (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\protocol\Connection.js:442:17)
    at Connection.execute (C:\Users\Mathew\Downloads\geometry-issue\node_modules\hdb\lib\protocol\Connection.js:623:8) {
  query: false
}
[error] - 500 > {
  code: '500',
  message: 'Cannot set parameter at row: 1. Wrong input for BINARY type'
}
[sql] - ROLLBACK

However, INSERT works correctly like so:

await INSERT({description: "Generated", geometry: "SRID=4326;POINT (1.62965918650832 3.1377292657822693)"}).into(Headers);

Output (notice that it correctly uses TO_GEOMETRY function):

[sql] - BEGIN
[sql] - INSERT INTO demo_Headers (ID,description,geometry) WITH SRC AS (SELECT ? AS JSON FROM DUMMY UNION ALL SELECT TO_NCLOB(NULL) AS JSON FROM DUMMY)
      SELECT ID AS ID,description AS description,TO_GEOMETRY(geometry) AS geometry FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(ID NVARCHAR(36) PATH '$.ID', "$.ID" 
NVARCHAR(2147483647) FORMAT JSON PATH '$.ID',description NVARCHAR(20000) PATH '$.description', "$.DESCRIPTION" NVARCHAR(2147483647) FORMAT JSON PATH '$.description',geometry NVARCHAR(2147483647) PATH '$.geometry', "$.GEOMETRY" NVARCHAR(2147483647) FORMAT JSON PATH '$.geometry') ERROR ON ERROR) [
  [
    {
      description: 'Generated',
      geometry: 'SRID=4326;POINT (1.62965918650832 3.1377292657822693)',
      ID: 'd0f16f3f-3c80-40e5-b2e0-b3df6aa8973f'
    }
  ]
]
[sql] - COMMIT

This is causing error during SAVE of draft-enabled entities when the CAP framework tries to update the active entity with geometries from the draft entity (without custom UPDATE call). It seems like HANAService.js's onUPDATE does not correctly set the UPDATE statement with TO_GEOMETRY(). Converting the geometry to buffer to pass to UPDATE like so works without errors:

await UPDATE(req.subject).with({geometry: wkx.Geometry.parse("SRID=4326;POINT (1.62965918650832 3.1377292657822693)").toWkb()});

Additional Information:

Detailed steps to reproduce

  1. git clone https://github.com/mathew-rizing/geometry-issue.git
  2. npm install
  3. cds deploy --to hana:
  4. npm run serve
  5. Open localhost:4004 > Open Preview for Headers entity
  6. Press Create with Geometry button on List Report, no errors
  7. Press Go to refresh, navigate to the new entry
  8. Press Update Geometry, error occurs
  9. Press Edit, press Save, error occurs

Details about your project

geometry-issue https://github.com/mathew-rizing/geometry-issue
@cap-js/asyncapi 1.0.2
@cap-js/cds-types 0.6.5
@cap-js/db-service 1.13.0
@cap-js/hana 1.3.0
@cap-js/openapi 1.0.6
@cap-js/sqlite 1.7.3
@sap/cds 8.3.0
@sap/cds-compiler 5.3.0
@sap/cds-dk (global) 8.3.0
@sap/cds-fiori 1.2.7
@sap/cds-foss 5.0.1
@sap/cds-mtxs 2.2.0
@sap/eslint-plugin-cds 3.1.0
Node.js v20.11.1
BobdenOs commented 1 month ago

Investigation

Currently the ST_GEOMETRY and ST_POINT functionalities are not fully supported on @cap-js/postgres and @cap-js/sqlite. There has been an initial attempt to make ST_POINT work in an uniform manor across databases, but this takes an javascript centric perspective on the API. Where a point is represented by an JSON of structure {x:1,y:1}. This implementation is capable of making all the databases behave the same, but clearly doesn't actually match to any user currently using ST types on HANA.

For postgres there is a widely supported extension that implements proper geo spatial types (aws,azure,gcp)

For sqlite there is also spatialite, but extensions for sqlite are always difficult to actually adopt.

Proposal

The current ST_POINT implementation will be removed and will rely on the native functionalities of the database to parse WKT and convert any native storage formats back into WKT when reading these fields.

Which will mean that for both ST_POINT and ST_GEOMETRY the input converters will have to be applied to UPDATE placeholders as they will bridge the gap between the natively stored value and the read values.

Related

The ST types are not the only type which requires the input converters for UPDATE placeholders. There is also the cds.vector type which uses input and output converters to make them work, but the input converter is currently also not applied to these UPDATE queries.

mathew-rizing commented 1 month ago

thanks for the investigation, that would also explain why I was experiencing issues with saving drafts on entities with cds.Vector only if they already have an active entity

BobdenOs commented 1 month ago

@mathew-rizing It should be fixed with the next release (test)

mathew-rizing commented 2 weeks ago

confirmed to be fixed with latest version, thanks!