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

Fix inconsistent behaviour across adapters with read types and "create" command #432

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

Conversation

katafrakt
Copy link

@katafrakt katafrakt commented Oct 27, 2024

During the investigation for #423 I discovered that for SQLite and MySQL (or basically for everything aside from PostgreSQL) the read types are applied twice. This works well if the read type allows it, but if it does not, it fails for these adapters, making behaviour between adapter inconsistent.

The solution here is to use relation.dataset.where instead of relation.where in default implementation of insert method of Create command. relation.where already applies the read types on materialization, while relation.dataset.where does not. This is also consistent with how Update command behaves.

With that in place, we always call finalize on "raw" data from the database, avoiding double application of a read type.

The PR consists of two commits. First one just adds tests replicating the problem. The second one just changes the code.

Similarily to what Update command do, use `dataset` to load the tuple
after `insert` in Create command. This way it loads "raw" data, without
applying read types yet. So when tuples are passed into `finalize` hook,
it can properly apply these read types.
@katafrakt
Copy link
Author

These CI failures look weird. Did my changes cause them?

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.

1 participant