cap-js / cds-dbs

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

[REGRESSION] `db-service v1.12.1` broke deep inserts #811

Closed hakimio closed 3 days ago

hakimio commented 2 weeks ago

Description of erroneous behavior

Following works fine with 1.12.0 but breaks with 1.12.1:

const result = await INSERT.into('MyService.MyEntity').entries({
  foo: 'bar',
  nestedEntity: {
    bar: 'baz'
  }
});

With 1.12.0 proper result is returned, with 1.12.1 result is 0 and nothing is inserted.

Details about your project

Name Version
OData version v4
Node.js version v20.13.1
@sap/cds 7.9.4
@sap/cds-compiler 4.9.8
@sap/cds-dk 7.9.7
@cap-js/postgres 1.10
hakimio commented 2 weeks ago

@David-Kunz

David-Kunz commented 1 week ago

Hi @hakimio ,

Can you kindly show us your model? The following example works as expected:

namespace myNamespace;
using { cuid } from '@sap/cds/common';

entity MyEntity : cuid {
  name: String(100);
  description: String(500);
  nestedEntity: Composition of one NestedEntity;
}

entity NestedEntity : cuid {
  name: String(100);
  value: Integer;
}
module.exports = srv => {

  srv.on('READ', 'MyEntity', async (req, next) => {

    const res = await INSERT.into('myNamespace.MyEntity').entries({
      ID: 'a11fb6f1-36ab-46ec-b00c-d379031e817a',
      nestedEntity: { ID: 'b22fb6f1-36ab-46ec-b00c-d379031e817a', value: 5 }

    })

    console.log(res) // InsertResult { results: [ { changes: 1, lastInsertRowid: 1 } ] }

    console.log(await SELECT.from('myNamespace.MyEntity', { ID: 'a11fb6f1-36ab-46ec-b00c-d379031e817a' })) // exists

    return next()
  })

}

Thanks and best regards, David

hakimio commented 1 week ago

The nestedEntity does NOT require an ID when the parent entity is created.

entity MyEntities: cuid {
    foo: String;
    nestedEntity: Composition of NestedEntities;
}

entity NestedEntities: cuid {
    bar: String;
}

This does not work anymore (no ID for nested or parent entity):

const result = await INSERT.into('MyService.MyEntities').entries({
  foo: 'bar',
  nestedEntity: {
    bar: 'baz'
  }
});

cuid

David-Kunz commented 1 week ago

Hi @hakimio ,

Your example works too:

using { cuid } from '@sap/cds/common';

entity MyEntities: cuid {
    foo: String;
    nestedEntity: Composition of NestedEntities;
}

entity NestedEntities: cuid {
    bar: String;
}
using { MyEntities } from '../db/schema';

service Foo {
  entity SMyEntities as projection on MyEntities;
}
module.exports = srv => {

  srv.before('READ', 'SMyEntities', async req => {
    const x = await INSERT.into('MyEntities').entries({
      nestedEntity: {
        bar: 'bar'
      }
    })
    const y = await INSERT.into('MyEntities').entries({})
    console.log(await SELECT.from('MyEntities', x[0])) // finds the entry
    console.log(await SELECT.from('MyEntities', y[0])) // finds the entry
  })

}

Can you kindly adjust the example to reproduce your issue?

Thanks and best regards, David

hakimio commented 1 week ago

@David-Kunz what version of @sap/cds are you using? Have you tested with v7? What db implementation are you using (type and version)?

David-Kunz commented 5 days ago

Hi @hakimio ,

I can reproduce it with @sap/cds 7. Will have a look.

David-Kunz commented 5 days ago

Sorry, @hakimio , it works, also with version 7. Can you reproduce it also with SQLite or does it only occur with Postgres?

hakimio commented 5 days ago

I only tested with Postgres. I'll try to debug and see if I can find out what exactly is going on sometime this week.

David-Kunz commented 5 days ago

Also can't reproduce it with Postgres.

cds version

@cap-js/cds-types: 0.2.0
@cap-js/db-service: 1.12.1
@cap-js/hana: 1.2.0
@cap-js/postgres: 1.10.0
@cap-js/sqlite: 1.7.3
@sap/cds: 7.9.5
@sap/cds-compiler: 4.9.8
@sap/cds-fiori: 1.2.7
@sap/cds-foss: 5.0.1
@sap/cds-mtxs: 2.0.0
Node.js: v22.9.0
David-Kunz commented 5 days ago

However, x === 0, even though it's inserted.

hakimio commented 5 days ago

That's still a bug.

David-Kunz commented 5 days ago

Yes, it was fixed with https://github.com/cap-js/cds-dbs/pull/803

David-Kunz commented 5 days ago

Let me check when we can release this.

David-Kunz commented 5 days ago

Will soon be released.

David-Kunz commented 5 days ago

And just to be sure: Is your data really not inserted? Can you double check with DEBUG=sql?

hakimio commented 5 days ago

Most likely what was happening was when the result was 0 instead of expected object, my app would throw an error and the transaction for the request would be reverted.

David-Kunz commented 5 days ago

I see, this will be fixed in the next version.

hakimio commented 3 days ago

Fixed in 1.13.0.