EXPOSED-372 UpsertStatement.resultedValues contains incorrect value

[obabichevjb]

Here is the fix for the issue EXPOSED-372

The problem is that upsert() command returns a wrong id in the case of UUIDTable.

The key reason for that difference I found is that for that table the uuid(columnName).autoGenerate().entityId() column is used, but inside InsertStatement there is a condition col.columnType.isAutoInc || value is NextVal<*> for overwriting the value from arguments.

I added col.defaultValueFun != null to the condition to avoid hard overwriting of the value from the result set. It fixes the problem from the issue.

But I feel that something wrong happens there, because it's weird to me that we need to rewrite values from result stet at all. I tried to replace the whole block:

if (col.columnType.isAutoInc || value is NextVal<*> || col.defaultValueFun != null) {
    map.getOrPut(col) { value }
} else {
    map[col] = value
}

with the statement map.getOrPut(col) { value } assuming that we take values from arguments only if it's not in the result set.

But after such change, some other tests start to fail (particularly for SQLite), because of the line above:

if (returnedValues.isEmpty() && firstAutoIncColumn != null) returnedValues[firstAutoIncColumn] = rs.getObject(1)

The value returned from rs.getObject(1) is not actual id, but something different. The test InsertTests::testInsertWithPredefinedId() fails on assertEquals(id1, entityID) with error Failed on SQLITE expected:<1> but was:<id1>. What means that rs.getObject(1) returned 1 instead of id1 (actual value in DB at this moment is id1).

[obabichevjb]

It looks like it's the wrong way to fix it, because other tests fail with Unexpected value of type UUID: 1 of kotlin.Int. I will try to find another way.

[obabichevjb]

It works now but particularly for Postgres. Using upsert command with other DBs will cause the same problem of returning wrong ID. It may confuse users.

[bog-walk]

For the future, there are some available sets for common TestDB collections in its companion object, if you ever need them.

[bog-walk]

From my understanding, PostgreSQL will be the only database that returns a result set populated with inserted/updated values that are not auto-incrementing. It might be good to investigate later if we can instead return a null resultValues for those databases, instead of an incorrect UUID. But I'm not sure how feasible that is as it would mean needing to determine whether the upsert was successful as an insert or an update (which I believe is only possible with MySQL), or risk breaking the expected behavior of inserts having their result sets overriden.

[e5l]

@bog-walk, if we're returning invalid UUID for some DBs it should be fixed before merging