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

delete on tables with rules #130

Closed
qwesda opened this Issue Nov 20, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@qwesda

qwesda commented Nov 20, 2015

I have a table where I have a rule like this:

CREATE OR REPLACE RULE soft_delete AS
ON DELETE TO "frontend_to_product"
WHERE old."comitted_status" IS NOT NULL
DO INSTEAD
UPDATE "frontends"."frontend_to_product" f_p
SET "desired_status" = NULL, "mod_hash" = NULL
WHERE (f_p."id_frontend", f_p."id_product") = (old."id_frontend", old."id_product");

the goal of this is to delete only items that have a committed status of NULL and otherwise set the desired_status to NULL.
When I want to delete rows via the table view postico generates the following query:

DELETE FROM "frontend_to_product" 
WHERE "id_frontend"='666040' AND "id_product"='14575';

but when I want to save the changes I get the error "Could not save changes – The query did not affect the expected number of rows. The transaction was rolled back."
The DELETE query returns 0 as the number of rows affected and postico gets confused.
A button to tell postico that everything is fine an the transaction should not be rolled back would helpful.

As a side note:
On tables with multi-column PRIMARY KEYS the generated query might be better readable if generated like this:

DELETE FROM "frontend_to_product" 
WHERE ("id_frontend", "id_product") = ('666040', '14575');

@jakob jakob added the started label Nov 22, 2015

@jakob

This comment has been minimized.

Show comment
Hide comment
@jakob

jakob Nov 22, 2015

Owner

Good point, thanks for the detailed explanation.

I've just added a button to commit the transaction anyway:
https://eggerapps-downloads.s3.amazonaws.com/postico-1254.zip

I've also added a hidden preference to disable the check altogether. Type the following command in Terminal if you want Postico to just ignore this error:
defaults write at.eggerapps.Postico IgnoreAffectedRows 1

Owner

jakob commented Nov 22, 2015

Good point, thanks for the detailed explanation.

I've just added a button to commit the transaction anyway:
https://eggerapps-downloads.s3.amazonaws.com/postico-1254.zip

I've also added a hidden preference to disable the check altogether. Type the following command in Terminal if you want Postico to just ignore this error:
defaults write at.eggerapps.Postico IgnoreAffectedRows 1

@jakob

This comment has been minimized.

Show comment
Hide comment
@jakob

jakob Nov 22, 2015

Owner

Concerning your suggestion for writing the query, is it always possible to write queries in that way? Is the row constructor syntax equivalent to using AND? The PostgreSQL documentation contains this puzzling quote: "Every row element must be of a type which has a default B-tree operator class" (see here), so I'm not sure I can always write the query like you suggested. (I don't want to change it since the way I curently do it seems to work)

Owner

jakob commented Nov 22, 2015

Concerning your suggestion for writing the query, is it always possible to write queries in that way? Is the row constructor syntax equivalent to using AND? The PostgreSQL documentation contains this puzzling quote: "Every row element must be of a type which has a default B-tree operator class" (see here), so I'm not sure I can always write the query like you suggested. (I don't want to change it since the way I curently do it seems to work)

@qwesda

This comment has been minimized.

Show comment
Hide comment
@qwesda

qwesda Nov 23, 2015

the new build works for me.

I understand the "don't change it, if it works" argument and one doesn't look at the generated sql statement so often anyway.

I think it does work all the time since a primary key is a NOT NULL constraint for every column of a multi column index and a UNIQUE constraint for the tuple.

It took me a while to find it but I think the relevant part of the documentation is this one:

Currently, only B-tree indexes can be declared unique.
http://www.postgresql.org/docs/9.4/static/indexes-unique.html

As I understand it a B-tree index implies a B-tree operator for every column (not sure about the default part).

qwesda commented Nov 23, 2015

the new build works for me.

I understand the "don't change it, if it works" argument and one doesn't look at the generated sql statement so often anyway.

I think it does work all the time since a primary key is a NOT NULL constraint for every column of a multi column index and a UNIQUE constraint for the tuple.

It took me a while to find it but I think the relevant part of the documentation is this one:

Currently, only B-tree indexes can be declared unique.
http://www.postgresql.org/docs/9.4/static/indexes-unique.html

As I understand it a B-tree index implies a B-tree operator for every column (not sure about the default part).

@jakob

This comment has been minimized.

Show comment
Hide comment
@jakob

jakob Nov 23, 2015

Owner

You are right, it probably works when there's a primary key. I'll leave it as is though, because the same code path also handles the case when all columns are compared (for tables without a primary key). Also, I remember there were some issues with row constructors on an old version of PostgreSQL, but I don't remember the details.

The new commit behavior is now released in version 1.0.1
https://eggerapps.at/postico/changelist.html

Owner

jakob commented Nov 23, 2015

You are right, it probably works when there's a primary key. I'll leave it as is though, because the same code path also handles the case when all columns are compared (for tables without a primary key). Also, I remember there were some issues with row constructors on an old version of PostgreSQL, but I don't remember the details.

The new commit behavior is now released in version 1.0.1
https://eggerapps.at/postico/changelist.html

@jakob jakob closed this Nov 23, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment