Skip to content
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

Deep UPDATE loses track of relationships #411

Open
BobdenOs opened this issue Jan 12, 2024 · 4 comments
Open

Deep UPDATE loses track of relationships #411

BobdenOs opened this issue Jan 12, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@BobdenOs
Copy link
Contributor

BobdenOs commented Jan 12, 2024

Description of erroneous behaviour

When doing a deep UPDATE when using an ambiguous WHERE clause it creates children without parents.

Detailed steps to reproduce

await INSERT([{ ID: 1 }, { ID: 2 }]).into(Genres)
const changes = await UPDATE(Genres)
 .where(`ID > `, { val: 0 })
 .with({
  children: [{ ID: 3 }, { ID: 4 }, { ID: 5 }]
})
expect(changes | 0).to.be.eq(2)

const finalState = await CQL`SELECT *,children{*} from ${Genres}`
const result = [
  { "ID": 1, "children": [] }, // Does not have children
  { "ID": 2, "children": [] }, // Does not have children
  // All children are created, but don't have any parent
  { "ID": 3, "children": [] },
  { "ID": 4, "children": [] },
  { "ID": 5, "children": [] },
]

Details about your project

package version
@sap/cds 7.5.x
@cap-js/db-service all
@BobdenOs BobdenOs added the bug Something isn't working label Jan 12, 2024
@johannes-vogel
Copy link
Contributor

I think that this is acceptable. If there's no clear value to be set, this must be set by the application.
We could discuss whether this should cause an error instead.

@BobdenOs
Copy link
Contributor Author

As deep updates like this can only be done to compositions it does not make sense to allow for updates that have / could result in multiple parents.

// allowed as it guarantees a single parent
UDPATE(Genres, {ID: 1}).with({children:[...]})
// Could be allowed as the where clause strictly checks all keys
UPDATE(Genres).where(`ID = `, 1).with({children:[...]})
// Could be rejected as there is the chance it will match multiple parents
UPDATE(Genres).with({children:[...]})
UPDATE(Genres).where(`ID != `, 0).with({children:[...]})
... etc

@BobdenOs BobdenOs self-assigned this Jan 12, 2024
@danjoa
Copy link
Contributor

danjoa commented Jan 29, 2024

These work?:

UPDATE(Genres) .where({ID:1}) .with({ children: [{ ID: 3 }, { ID: 4 }, { ID: 5 }] })
UPDATE(Genres,1) .with({ children: [{ ID: 3 }, { ID: 4 }, { ID: 5 }] })

@patricebender
Copy link
Member

is this still relevant? We reworked some of the deep logic in the past weeks @David-Kunz ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants