Fix 'NULL DEFAULT _' migrations. #380
Conversation
|
what about the UPSERT operation that is called when another device makes a change. What would happen if a device on an older version makes a change and it is synced to a device with the new schema. What column names are passed to |
|
Hi @hishnash, good catch. We pushed a few more changes to cover another edge case and wrote some tests to demonstrate it. Can you let us know if this works for you? |
| else { | ||
| return "" | ||
| } | ||
| let hasNonPrimaryKeyColumns = columnNames.contains { $0 != T.primaryKey.name } |
There was a problem hiding this comment.
is it possible that columnNames here somehow ends up with the column name that is not in the filtered allColumnNames and thus would result in a SQL error as columnNames is used in the ON CONFLICT DO SET option. Maybe columnNames need to drop any names that are not found in allColumnNames (I know there is some other method doing this further up before upsert is called but that method does not consider the hasSet(key: $0) )
There was a problem hiding this comment.
Yeah, that seems theoretically possible, but haven't thought of the exact scenario where that might happen. I pushed an updated.
This is an alternative fix to #362
When performing a migration of the form "ADD COLUMN … DEFAULT _" (i.e. a nullable column), then we want the default to be inserted into the local database and stay there after the sync engine has fully started up. Currently there is a bug where that data gets NULL'd out after start up, and this PR fixes that.