Redundant changelog entries for decimal fields on HANA

stockbal commented 3 months ago

Hi,

we are currently using the change-tracking plug-in in a CAP project that is deployed on cloud foundry and uses SAP HANA Cloud as a database. Locally with SQLite every works pretty well, but after deployment we see many change entries where numeric values have not really changed.

npm versions

@cap-js/change-tracking@1.0.7
@cap-js/sqlite@1.7.3
@sap/cds-dk@7.9.7
@sap/cds-hana@2.0.0
@sap/cds@7.9.4

Defined cds entity

entity MyEntity : cuid {
    busID        : String(28) not null;
    numericField : Decimal(11, 4) not null;
}

Example 1: Update operation via OData service

PATCH {{server}}/odata/v4/demo/MyEntityProjection(8f9a1a95-5cc9-4274-8252-52ed358d9d71)
Content-Type: application/json

{
  "numericField": 3000
}

{
  ...
  "attribute": "numericField",
  "valueChangedFrom": "3000.0000",
  "valueChangedTo": "3000",
  "valueDataType": "cds.Decimal",
  "modification": "update",
  "entityID": "1111",
  ...
}

Example 2: Update operation via CQL query on projection

await UPDATE.entity(MyEntityProjection, "1111").set({numericField: 3000});

{
  ...
  "attribute": "numericField",
  "valueChangedFrom": "3000.0000",
  "valueChangedTo": 3000,
  "valueDataType": "cds.Decimal",
  "modification": "update",
  "entityID": "1111"
  ...
}

As you can see, the issue can be traced to the HANA driver always filling up the defined decimal places with zeros. In case of sending 3.5 to the database, you will get back a string of value "3.5000" if selecting the entry again.

One fix could be to always cast both old and new value to Number and compare that, in case the data type is cds.Decimal.

Regards, Ludwig

nkaputnik commented 2 months ago

Hello @stockbal we have picked this up and will be working on it.

Best, Nick

Sv7enNowitzki commented 2 months ago

Hi @stockbal

We believe that this issue is one that the CAP framework needs to address, as it is essentially due to the differences in behavior among databases, which are then reflected in the req.diff API.

Perhaps we should communicate with the CAP team to resolve this, addressing the problem at the framework level rather than at the plugin level.

I have already submitted an issue to the CAP team and am currently in communication with them: https://github.tools.sap/cap/issues/issues/16878

Best Regards, Wenjun

Sv7enNowitzki commented 1 month ago

Hi @stockbal

After discussing with the CAP team, as they mentioned in their issue, there indeed are such problems with the current req.diff. They are currently working on a new solution.

If we want to solve similar problems, for now, we can only address them within the project itself. Therefore, I have submitted a PR, which has already been merged into the main branch. Thank you for your patience. It will be available in next release.

Best Regards, Wenjun

cap-js / change-tracking