Skip to content

add test case for a beforeInsert hook on a composite primary key#8527

Closed
KampfCaspar wants to merge 1 commit into
pubkey:masterfrom
KampfCaspar:compositePrimary_beforeInsert
Closed

add test case for a beforeInsert hook on a composite primary key#8527
KampfCaspar wants to merge 1 commit into
pubkey:masterfrom
KampfCaspar:compositePrimary_beforeInsert

Conversation

@KampfCaspar
Copy link
Copy Markdown

This PR contains:

BUG TEST CASE

Describe the problem you have

I want to dynamically but centrally assign primary keys for new documents in a pre-insert hook. This works fine for scalar primary keys but fails for composite primary keys

Describe the solution

Best: fields for a composite primary key can bet set in a pre-Hook.
Other: primary key fields (irrespective of primary key type) are taboo.

schema: schemaComposite
}
});
collections.collectionScalar.preInsert( (data) => {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

why do you use two different hooks? I think you need a single one that asigns id, name and number at the same time.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If I understood the documentation correctly, I should only set the composite fields of a composite primary key. The 'primary key field' - while I seem to have to define it myself - should be set by rxdb itself (using the composite fields).

Therefore in the scalar example, I directly set the id field - whereas in the composite example, I set name and number (and rxdb should combine them into id itself).

The two pre-insert hooks fill different fields.

@KampfCaspar
Copy link
Copy Markdown
Author

I researched the case further and found a further complication:

It seems in fillObjectDataBeforeInsert (line 40) tries to fill in the primary key field (before the pre-insert hook) but ONLY on composite primary keys - ordinary scalar (string) primary key fields are left alone. This results in a scalar primary key being fillable by the pre-insert hook, whereas a composite primary key bails out before the hook is called.

now insert/bulkInsert and bulkUpsert (which calls bulkInsert) could be easily be adapted, but bulkUpsert calls fillObjectDataBeforeInsert effectively twice and checks the existence of (any) primary key itself... Might it be prudent to split the 'document preparation' from the actual 'document insertion'?

incerementalUpsert, however, uses the primary key for further concurrency checks. Therefore it definitely needs the 'combined' composite primary key even earlier - definitely before pre-insert is run.

If setting primary key fields in a pre-hook should be supported, it seems a separate 'document preparation hook' to be called even before the pre-insert hook might be prudent?

@pubkey
Copy link
Copy Markdown
Owner

pubkey commented May 19, 2026

Fixed in #8550

Please test.

@KampfCaspar
Copy link
Copy Markdown
Author

works fine. Thx.

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