Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing pk column is not supported #238

Closed
olsen232 opened this issue Sep 2, 2020 · 2 comments · Fixed by #740
Closed

Changing pk column is not supported #238

olsen232 opened this issue Sep 2, 2020 · 2 comments · Fixed by #740

Comments

@olsen232
Copy link
Collaborator

olsen232 commented Sep 2, 2020

In the V2 structure, changing which column is the primary key means that every feature has to be deleted and rewritten in a new location.
Preliminary tests indicate this works to some extent, but definitely the diff shown for this change will be useless. We need to do more work if we want to support this properly.

@olsen232
Copy link
Collaborator Author

olsen232 commented Jul 7, 2021

This actually seems to work about as well as can be expected. Kart treats a feature as the same if it has the same primary key - all primary keys changing at once means everything old was deleted, and everything new is newly inserted. But, that's consistent with how kart works.

@olsen232 olsen232 closed this as completed Jul 7, 2021
@olsen232 olsen232 reopened this Nov 16, 2022
@olsen232
Copy link
Collaborator Author

This is not done, so closing this bug was an oversight.
The limited but reasonable-considering-Kart's-model behaviour described above does happen if you manage to create a commit where the primary key changes. This is possible to do using kart import which is okay with such changes.

It is not possible to create such a commit by modifying the working copy, since there is a check preventing it. Attempting to modify the working copy to change the primary key column will fail at the commit-step with "Schema changes that involve primary key changes are not yet supported".

Fixing this so that it works just as it does at import-time is of course possible, but will just need to take an extra step of explicitly deleting every pre-existing row from the model and explicitly adding every current row from the working copy during any commit which changes the primary key, since the DB operation which changed the primary key won't necessarily have caused every single row to be marked as dirty, but every row is dirty since it potentially (probably) has a new PK value and so needs to be deleted from one path and rewritten at another path.

Note that changing primary key type from int to text or vice versa would also require that the path metadata is rewritten, which is also not yet implemented.

olsen232 added a commit that referenced this issue Nov 30, 2022
Note that changing the PK column disconnects every past feature
from every future feature, since the way we know that a feature
is the same feature going forward is that it continues to have the same
primary key value.
olsen232 added a commit that referenced this issue Dec 1, 2022
Fix for #238 - changing PK column (was not supported)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant