sql: INSERT..ON CONFLICT updates columns not in UPDATE clause

[cuongdo]

Repro:

1. Create a table and insert a row into it:

CREATE DATABASE IF NOT EXISTS test;

CREATE TABLE IF NOT EXISTS test."users" ("id"  SERIAL , "name" VARCHAR(255), "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL, "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL, PRIMARY KEY ("id"));

INSERT INTO test."users" ("id","name","createdAt","updatedAt") VALUES (1,'original','2017-03-07 16:05:50.924 +00:00','2017-03-07 16:05:50.924 +00:00');

SELECT * FROM test.users;
+----+----------+-------------------------------+-------------------------------+
| id |   name   |           createdAt           |           updatedAt           |
+----+----------+-------------------------------+-------------------------------+
|  1 | original | 2017-03-07 16:05:50.924+00:00 | 2017-03-07 16:05:50.924+00:00 |
+----+----------+-------------------------------+-------------------------------+

2. Then do an upsert that updates all columns except createdAt:

INSERT INTO test."users" ("id","name","createdAt","updatedAt") VALUES (1,'UPDATED','2017-03-07 16:05:51.946 +00:00','2017-03-07 16:05:51.946 +00:00')
ON CONFLICT (id) DO UPDATE SET "id" = excluded."id", "name" = excluded."name", "updatedAt" = excluded."updatedAt";

SELECT * FROM test.users;
+----+---------+-------------------------------+-------------------------------+
| id |  name   |           createdAt           |           updatedAt           |
+----+---------+-------------------------------+-------------------------------+
|  1 | UPDATED | 2017-03-07 16:05:51.946+00:00 | 2017-03-07 16:05:51.946+00:00 |
+----+---------+-------------------------------+-------------------------------+

SHA: e66b654
Actual: both createdAt and updatedAt are updated
Expected: only updatedAt is updated

I've traced the issue to this line:
https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/tablewriter.go#L235

(*tableUpserter).init() is terminating very early. When I remove that whole if block, the upsert works correctly. There are several things I don't understand here, so assigning this to @danhhz.

Also, this scenario needs a logic test.

[cuongdo]

This seems to fix the issue, but I don't know what else this might break:

diff --git a/pkg/sql/tablewriter.go b/pkg/sql/tablewriter.go
index 5b34be0..bf1e828 100644
--- a/pkg/sql/tablewriter.go
+++ b/pkg/sql/tablewriter.go
@@ -227,6 +227,7 @@ func (tu *tableUpserter) init(txn *client.Txn) error {
        tu.indexKeyPrefix = sqlbase.MakeIndexKeyPrefix(tu.tableDesc, tu.tableDesc.PrimaryIndex.ID)

        allColsIdentityExpr := len(tu.ri.insertCols) == len(tu.tableDesc.Columns) &&
+               len(tu.updateCols) == len(tu.tableDesc.Columns) &&
                tu.evaler != nil && tu.evaler.isIdentityEvaler()
        // When adding or removing a column in a schema change (mutation), the user
        // can't specify it, which means we need to do a lookup and so we can't use

[danhhz]

Just took a look at this; this query definitely shouldn't be fast pathed. Your fix seems to me like the correct one, but I want to briefly remind myself how it worked when I wrote it (before the various intervening refactors) before I commit to that.

[danhhz]

This was always broken, thanks for tracking it down, @cuongdo! I'm sending a fix shortly

[cuongdo]

This is happening again as of 1758050.

Same repro as above.

[bdarnell]

#14034 was included in the draft release notes in cockroachdb/docs#1214. Does it need to be removed from the release notes for this week?

[danhhz]

Ah, I see. The "id" = excluded."id" is throwing it off. It's a little wonky to include a column in the conflict index in the SET clause, but I can see how it would happen. Fix coming probably sometime today

[cuongdo]

Yes, it is a little strange, but unfortunately, generated queries such as those from ORMs do have some oddities.