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

feat: deep UPDATE without reading #866

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

BobdenOs
Copy link
Contributor

@BobdenOs BobdenOs commented Oct 24, 2024

tests are currently failing only on HXE everything passed on HCE

@@ -214,7 +214,7 @@ class SQLService extends DatabaseService {
// REVISIT: It's not yet 100 % clear under which circumstances we can rely on db constraints
return (super.onDELETE = /* cds.env.features.assert_integrity === 'db' ? this.onSIMPLE : */ deep_delete)
async function deep_delete(/** @type {Request} */ req) {
let { compositions } = req.target
let { compositions } = (req.target ??= req.query.target)
Copy link
Contributor

@David-Kunz David-Kunz Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when is this not set? /cap/cds/blob/402a114f8968c2bc25c5f1e2de2208f4cac0c66d/lib/srv/srv-dispatch.js#L29

@@ -750,7 +751,7 @@ class CQN2SQLRenderer {
const managed = this._managed.slice(0, columns.length)

const extractkeys = managed
.filter(c => keys.includes(c.name))
// .filter(c => keys.includes(c.name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Comment on lines +1014 to +1015
if (val[val.length - 1] === ',') val = arg.json = val.slice(0, -1) + ']'
if (val[val.length - 1] === '[') val = arg.json = val + ']'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (val[val.length - 1] === ',') val = arg.json = val.slice(0, -1) + ']'
if (val[val.length - 1] === '[') val = arg.json = val + ']'
if (val.at(-1) === ',') val = arg.json = val.slice(0, -1) + ']'
if (val.at(-1) === '[') val = arg.json = val + ']'

-- DEBUG => this.dbc._native.prepare(cds.utils.fs.readFileSync(__dirname + '/deep-genres.sql','utf-8')).exec([JSON.stringify(query.UPDATE.data)])
DO (IN JSON NCLOB => ?) BEGIN

-- Extract genres with a depth of 3 (like: '$.children[*].children[*]')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depth can be determined based on input json, right? we must just union all all provided rows

ERROR ON ERROR
) AS NEW;

-- DELETE all children of parents that are no longer in the dataset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are rows deleted which are not children of parents, but which are not part of the dataset?

E.g.

JSON input:

ID parent_ID
1 null
11 1
12 1
2 null
21 2
22 2

Existing data:

ID parent_ID will be deleted?
13 1 yes
3 null no

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: They might be children of other entities and would not have to be deleted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about it again, we can exclude non-children entities since compositions, by definition, are children and one needs to consider only children and children's children of the root instance.

We should also look at the case of a composition of one where the foreign key is part of the parent. Here, one cannot identify the previous child purely based on the payload and one needs to take into account the foreign keys of existing data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants