Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#7203 closed defect (fixed)

Carto: Double upload results in sequence errors

Reported by: pramsey Owned by: warmerdam
Priority: normal Milestone: 2.2.4
Component: OGR_SF Version: svn-trunk
Severity: normal Keywords:
Cc:

Description

Running this upload twice

ogr2ogr \
 --config DEBUG ON \
 --config CARTO_API_KEY xxxxxxxx \
 -t_srs EPSG:4326 \
 -f Carto \
 -nln pts_tmp \
 "Carto:pramsey" \ 
 pts2.shp

results in one success and then errors, one per record it seems:

ERROR 1: HTTP error code : 400
ERROR 1: RunSQL Error Message:HTTP error code : 400
ERROR 1: Error returned by server : relation "pts_tmp_cartodb_id_seq" does not exist

I'm wondering if this has something to do with the table creation in RunDeferredCreationIfNecessary? which both creates a serial column and then also creates and associates a sequence. Thought maybe that doesn't matter and we just get some orphaned sequences.

Attachments (2)

nextval.patch (1.9 KB) - added by pramsey 11 months ago.
Idea for fix
nextval.2.patch (2.5 KB) - added by pramsey 11 months ago.
Also use a non-colliding initial sequence name

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 months ago by Jukka Rahkonen

I think that you command is not a good one without either -append or -overwrite http://www.gdal.org/ogr2ogr.html. But perhaps you would like to see some other error message?

Last edited 11 months ago by Jukka Rahkonen (previous) (diff)

Changed 11 months ago by pramsey

Attachment: nextval.patch added

Idea for fix

comment:2 Changed 11 months ago by pramsey

The problem is that Carto is modifying the table away from ogr's expectation. OGR is assuming the primary key seq will always be named <table>_<column>_seq, and that's not true, as it looks like CDB_Cartodbfy() is changing out the sequence name at the end of the first load. I think that (a) ogr shouldn't make assumptions and (b) Carto shouldn't mess with valid sequences in CDB_Cartodbfy(), so here's a patch for (a) and I'll look at why (b) is happening on our side.

Last edited 11 months ago by pramsey (previous) (diff)

comment:3 Changed 11 months ago by pramsey

Isn't there a default mode? Using -overwrite works, I guess the table is cleanly dropped first. Explicitly using -append results in the errors, so I guess append is the default mode.

comment:4 Changed 11 months ago by Jukka Rahkonen

I guess that it depends on the driver. I think that user friendly and safe behavior would be to create a table straight ahead with -nln if table does not exist and throw an error if it does exist, with a hint to use explicitly either -append or -overwrite. Of course the default must not be -overwrite. Some drivers support also truncate, like PostGIS driver through a config option. Might be useful in Carto driver as well.

Actually it seems that in this case the default comes from the layer creation option "OVERWRITE" of the Carto driver http://www.gdal.org/drv_carto.html.

Last edited 11 months ago by Jukka Rahkonen (previous) (diff)

Changed 11 months ago by pramsey

Attachment: nextval.2.patch added

Also use a non-colliding initial sequence name

comment:5 Changed 11 months ago by pramsey

Using %s_%s_fid_seq as the sequence name should give the CDB_Cartodbfy() function the space it need to create a %s_%s_seq when it re-writes the table (as it must, since the table as created by ogr lacks a the_geom_webmercator). It's a bit of a guess, whether it's better to upload only one geometry and accept the overhead in CDB_Cartodbfy() or to write a perfect table (at the cost of uploading two copies of the geometry). A tricky cake-and-also-eat-it solution would be to create the table with a mercator column, and add the _CDB_update_the_geom_webmercator trigger before loading the data. That probably exposes too much Carto guts though, best not to do that.

comment:6 Changed 11 months ago by Even Rouault

Resolution: fixed
Status: newclosed

In 41286:

Carto: fix append mode by retrieving the sequence name for the primary key (fixes #7203)

comment:7 Changed 11 months ago by Even Rouault

In 41287:

Carto: fix append mode by retrieving the sequence name for the primary key (fixes #7203)

comment:8 Changed 11 months ago by Even Rouault

@pramsey Would be nice to provide patches that actually compile ;-)

I've fixed it differently by retrieving the sequence name.

The fact that "ogr2ogr -f carto" ran the second time proceeds to append mode is a bit of a side effect of the ogr2ogr logic and not completely intended. Normally you should do "ogr2ogr -append -t_srs EPSG:4326 -nln pts_tmp carto:pramsey pts2.shp"

comment:9 Changed 11 months ago by pramsey

Thank you Even, I should have labelled it "patch for discussion purposes" :)

Note: See TracTickets for help on using tickets.