-
Couldn't load subscription status.
- Fork 53
Sequence Rename to avoid Collision #319
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
Conversation
| INTO relseq; | ||
| -- If it's the name we want, then rename it | ||
| IF relseq IS NOT NULL AND relseq = Format('%I.%I', destschema, destseq) THEN | ||
| PERFORM _CDB_SQL(Format('ALTER SEQUENCE %s RENAME TO %s_tmp', relseq, destseq), '_CDB_Rewrite_Table'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must be missing something: it seems to me it's renaming the existing seq relseq to the desired destseq (_cartodb_id_seq) when it already has that very same name (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's moving %s_cartodb_id_seq to _cartodb_id_seq_tmp so that in the following steps, when the new table is created, it will be possible for the serial in cartodb_id to use %s_cartodb_id_seq as it's sequence name. Probably it would be best to recognize this case (sequence already there, and usable, even if the table is going to be re-written) as another case, but that would require larger changes below, to address what is a fairly minor corner case right now (the ogr2ogr upload issue).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason I was missing the _tmp 😅 thanks for the clarification!
|
A client is experiencing this same issue when using ogr2ogr to have a ETL workflow to append data to the same table when importing with ogr2ogr. Having a message error like |
|
Thank you guys for the fix and review. Aside from the merge, there's also release and deploy for users to enjoy it. @oriolbx can you please reference the issue here and add as candidate to the RT? |
|
We gave the affected user an alternative to install and build the version where there is the fix. The fix went into gdal 2.2, so they can work off a stable branch https://trac.osgeo.org/gdal/changeset/41287 |
This doesn't really fix anything on our side, since the existing code already avoids sequence name collisions, but it addresses the issue in #318. This would just allow older versions of ogr2ogr to avoid the bug fixed in 7203 where ogr2ogr fixates on having a serial sequence named
<relname>_cartodb_id_seq. What exposed the error wasogr2ogrdata upload (works)It happens because after the first
ogr2ogrupload, the table is cartodb'fyed, and the sequence name changes when cartodbfy does the copy step, so it ends up with a unique sequence name that is not the<relname>_cartodb_id_seqformogr2ogrexpects when running in-appendmode, which seems to be the default for a not-first upload.