Repository: codenotary/immudb (language: go)

Issue:
[SQL] ON CONFLICT DO NOTHING doesnt seem to work
<!-- Please use this template while reporting a bug and provide as much info as possible. 
If applicable, add shell logs or screenshots to help explain your problem.
Thanks! -->

**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)**



Discussion / hints:
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.

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...
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. 


> 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

Fix patch:
diff --git a/embedded/sql/stmt.go b/embedded/sql/stmt.go
index 5843bff9f7..fd57962865 100644
--- a/embedded/sql/stmt.go
+++ b/embedded/sql/stmt.go
@@ -692,7 +692,7 @@ func (stmt *UpsertIntoStmt) execAt(ctx context.Context, tx *SQLTx, params map[st
 
 			if err == nil && stmt.onConflict != nil {
 				// TODO: conflict resolution may be extended. Currently only supports "ON CONFLICT DO NOTHING"
-				return tx, nil
+				continue
 			}
 		}
 


Test patch:
diff --git a/embedded/sql/engine_test.go b/embedded/sql/engine_test.go
index 3af4e622f4..b93402df8f 100644
--- a/embedded/sql/engine_test.go
+++ b/embedded/sql/engine_test.go
@@ -839,7 +839,7 @@ func TestInsertIntoEdgeCases(t *testing.T) {
 	_, _, err := engine.Exec(context.Background(), nil, "CREATE TABLE table1 (id INTEGER, title VARCHAR[10], active BOOLEAN, payload BLOB[2], PRIMARY KEY id)", nil)
 	require.NoError(t, err)
 
-	_, _, err = engine.Exec(context.Background(), nil, "CREATE INDEX ON table1 (title)", nil)
+	_, _, err = engine.Exec(context.Background(), nil, "CREATE UNIQUE INDEX ON table1 (title)", nil)
 	require.NoError(t, err)
 
 	_, _, err = engine.Exec(context.Background(), nil, "CREATE INDEX ON table1 (active)", nil)
@@ -863,6 +863,23 @@ func TestInsertIntoEdgeCases(t *testing.T) {
 		require.Nil(t, ctxs[0].TxHeader())
 	})
 
+	t.Run("on conflict case with multiple rows", func(t *testing.T) {
+		ntx, ctxs, err := engine.Exec(context.Background(), nil, `
+			INSERT INTO table1 (id, title, active, payload)
+			VALUES
+				(1, 'title1', true, x'00A1'),
+				(11, 'title11', true, x'00B1')
+			ON CONFLICT DO NOTHING`, nil)
+		require.NoError(t, err)
+		require.Nil(t, ntx)
+		require.Len(t, ctxs, 1)
+		require.Equal(t, 1, ctxs[0].UpdatedRows())
+		require.NotNil(t, ctxs[0].TxHeader())
+
+		_, _, err = engine.Exec(context.Background(), nil, "INSERT INTO table1 (id, title, active, payload) VALUES (1, 'title11', true, x'00B1')", nil)
+		require.ErrorIs(t, err, store.ErrKeyAlreadyExists)
+	})
+
 	t.Run("varchar key cases", func(t *testing.T) {
 		_, _, err = engine.Exec(context.Background(), nil, "INSERT INTO table1 (id, title, active, payload) VALUES (2, 'title123456789', true, x'00A1')", nil)
 		require.ErrorIs(t, err, ErrMaxLengthExceeded)