codenotary / immudb

immudb - immutable database based on zero trust, SQL/Key-Value/Document model, tamperproof, data change history
https://immudb.io
Other
8.55k stars 343 forks source link

[SQL] ON CONFLICT DO NOTHING doesnt seem to work #1501

Closed fsvieira closed 1 year ago

fsvieira commented 1 year ago

What happened

I am trying to do a simple insert or ignore, but when at least one key exists it gives me the key exists error.

What you expected to happen

I would expect that records with no conflicts get inserted, and records with conflicts are just ignored.

How to reproduce it (as minimally and precisely as possible)

lets say I have a table parcel_cell where cell_wkt is unique:

INSERT INTO parcel_cell
            (
                cell_wkt,
                updated_at,
                created_at,
                updated_by,
                parcel_id
            )
            VALUES (
                'POINT(1 2)',
                NOW(),
                NOW(),
                317,
                103500
            ) ON CONFLICT DO NOTHING;

First time it inserts, next time gives an error because key already exists, since its only one there is no problem.

But if I do this:

INSERT INTO parcel_cell
            (
                cell_wkt,
                updated_at,
                created_at,
                updated_by,
                parcel_id
            )
            VALUES (
                'POINT(1 2)',
                NOW(),
                NOW(),
                317,
                103500
            ),
            (
                'POINT(3 4)',
                NOW(),
                NOW(),
                317,
                103500
            ) ON CONFLICT DO NOTHING;

It gives a error key exists, but 'POINT(3 4)' doesn't exists and it doesn't get inserted.

Environment I am using dashboard , and it says this: immudb: v1.4.1 ws: v1.0.18

Additional info (any other context about the problem)

jeroiraz commented 1 year ago

Hi @fsvieira, thanks for reporting it. I'll check it.

The second insertion case, it's expected to behave like that due to the atomicity of the insertion. All the records are inserted as part of the same transaction, so if an error was raised, the whole transaction won't be committed.

jeroiraz commented 1 year ago

I think the conflict is not in the primary key but on an indexed column with unique constraint, right?

If so, I think it's not evident how to handle the conflict, as then index won't be pointing to the newly inserted record...

fsvieira commented 1 year ago

I made a simpler test case

CREATE TABLE test2 (
  id INTEGER AUTO_INCREMENT,
  uv  INTEGER,
  PRIMARY KEY (id)
);

CREATE UNIQUE INDEX ON test2 (uv);

Inserting on a empty table, everything is created normally.

insert into test2 (id, uv)
values (1, 1), (2, 2), (3, 3) ON CONFLICT DO NOTHING;

Inserting again with one more value (4,4) , there is no error, but (4,4) is not inserted. I think in most database this would be inserted, INSERT OR IGNORE, INSERT IGNORE, or ON CONFLICT DO NOTHING .

insert into test2 (id, uv)
values (1, 1), (2, 2), (3, 3), (4, 4) ON CONFLICT DO NOTHING;

Now with only the unique value, most databases would ignore conflict constrains when you use a IGNORE OR ON CONFLICT DO NOTHING . So I think should have inserted the (4) , and ignored the other values.

insert into test2 (uv)
values (1), (2), (3), (4) ON CONFLICT DO NOTHING; 

Maybe I am wrong, but I think sqlite, mysql, postgres work that way.

I wanted to add ON CONFLICT DO UPDATE but I wasn't able to make it work, it says that expects NOTHING after DO.

Thanks.

jeroiraz commented 1 year ago

4, 4)

Thanks for the super detailed description. Aham, yes, in this case insertion of the fourth row should take place because there is no violation of the uniqueness constraint of the secondary index.

Thanks a lot for reporting it, we'll fix it asap

jeroiraz commented 1 year ago

@fsvieira it would be great if you can build immudb from branch fix_sql_onconflict and check if it's working as you would expect

fsvieira commented 1 year ago

Hi, sorry for the late response.

I was evaluating immudb and I decided not use it for now, not sure if I am able to build it and test it, I would make exactly the same test as I put in the description. But if needed I can try to test it.

Do you still want me to test it ?

jeroiraz commented 1 year ago

Hi, sorry for the late response.

I was evaluating immudb and I decided not use it for now, not sure if I am able to build it and test it, I would make exactly the same test as I put in the description. But if needed I can try to test it.

Do you still want me to test it ?

Hi @fsvieira, given immudb was not released yet but the fix was already merged into master branch. If you'd like, you can run the immudb docker image:

docker run -it -d -p 3322:3322 --name immudb codenotary/immudb:dev

We could have a chat on Discard channel (https://discord.gg/EWeCbkjZVu) when you have the time, I'd like to receive your feedback about immudb.