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

upsert of new row with check constraints fails #514

Closed
cldellow opened this issue Nov 26, 2022 · 5 comments
Closed

upsert of new row with check constraints fails #514

cldellow opened this issue Nov 26, 2022 · 5 comments

Comments

@cldellow
Copy link

(I originally opened this in simonw/datasette-insert#20, but I see that that library depends on sqlite-utils)

In the case of a new row, upsert first adds the row, specifying only its pkeys:

sql = "INSERT OR IGNORE INTO [{table}]({pks}) VALUES({pk_placeholders});".format(
table=self.name,
pks=", ".join(["[{}]".format(p) for p in pks]),
pk_placeholders=", ".join(["?" for p in pks]),
)

This means that a table with NON NULL (or other constraint) columns that aren't part of the pkey can't have new rows upserted.

@simonw
Copy link
Owner

simonw commented May 8, 2023

@simonw simonw closed this as completed May 8, 2023
@simonw
Copy link
Owner

simonw commented May 8, 2023

This means that a table with NON NULL (or other constraint) columns that aren't part of the pkey can't have new rows upserted.

Huh... on that basis, it's possible my fix in 2376c45 is incomplete. I only covered the 'not null' case.

@simonw simonw reopened this May 8, 2023
@simonw
Copy link
Owner

simonw commented May 8, 2023

OK, this fails silently:

import sqlite_utils
db = sqlite_utils.Database(memory=True)
db.execute('''CREATE TABLE employees (
    id INTEGER PRIMARY KEY,
    name TEXT,
    age INTEGER,
    salary REAL,
    CHECK (salary is not null and salary > 0)
);''')
db["employees"].upsert({"id": 1, "name": "Bob"}, pk="id")
list(db["employees"].rows)

It outputs:

[]

@simonw
Copy link
Owner

simonw commented May 8, 2023

Applying the fix from the PR here doesn't fix the above problem either:

So it looks like these kinds of check constraints currently aren't compatible with how upsert() works.

@simonw
Copy link
Owner

simonw commented May 8, 2023

Seeing as sqlite-utils doesn't currently provide mechanisms for adding check constraints like this I'm going to leave this - I'm happy with the fix I put in for the not null constraints.

@simonw simonw closed this as completed May 8, 2023
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 a pull request may close this issue.

2 participants