Add support to detect string cartodb_id columns

[iriberri]

Going to try adding more tests now.

[rafatower]

better noting this is traversing the table and therefore breaking the paradigm of the "rewrite" function: just do one pass after figuring it out how to transform into the intended target table.

[iriberri]

Related disabled tests:

cartodb-postgresql/test/CDB_CartodbfyTableTest.sql

Lines 167 to 179 in 54973c0

	-- INFO: disabled because cartodbfy does not longer consider text columns for primary ID
	-- -- table with existing cartodb_id field of type text
	-- CREATE TABLE t AS SELECT 10::text as cartodb_id;
	-- SELECT CDB_CartodbfyTableCheck('t', 'text cartodb_id');
	-- select cartodb_id/2 FROM t;
	-- DROP TABLE t;
	-- INFO: disabled because cartodbfy does not longer consider text columns for primary ID
	-- -- table with existing cartodb_id field of type text not casting
	-- CREATE TABLE t AS SELECT 'nan' as cartodb_id;
	-- SELECT CDB_CartodbfyTableCheck('t', 'uncasting text cartodb_id');
	-- select cartodb_id,_cartodb_id0 FROM t;
	-- DROP TABLE t;

[rafatower]

you can blame me :S and re-enable those tests

[iriberri]

Not blaming! text was not supported anymore so... :)

I reenabled and added an extra one for text column in an empty table, I'm still playing with it locally.

[rafatower]

I would use types sparingly. Alternatively the function can return a RECORD

[iriberri]

Agree :-) I had some issues late yesterday with the returns and one of my attempts was to force this, but it's not needed anymore.

[iriberri]

So:

• We're gonna asume that whatever cartodb_id we get is valid
• Due to this, we can just ignore the checks of type for the column cartodb_id and force in the Rewrite table function it's casting to big int. If cartodb_id doesn't follow the requirement this will just fail (fail = import failure)
• We can still maintain the int checks for columns not named cartodb_id. It might probably be worthy to ignore them and generate a new cartodb_id from scratch, but not the point right now

[iriberri]

Okay, if I didn't miss anything again, this is now more aligned.

I didn't edit at all the branch of "if there's another valid column not named cartodb_id..." as we said and as I wrote in my last comment.

[iriberri]

Wondering if I could make the CartoDBfy test function to catch these errors and output a less verbose error instead of performing tests against this specific context backtrace.

[iriberri]

Answering to myself: I set verbosity to terse to avoid contexts.

[rafatower]

👍 time to prepare a release

[iriberri]

From @rafatower's feedback, add test:

cartodbfy(t) = cartodbfy(cartodbfy(t))

[iriberri]

Some inconsistency here:

Scenario A:

CREATE TABLE table_50(
    cartodb_id    varchar,
    title         varchar(40)
);
INSERT INTO table_50 (cartodb_id) VALUES ('1');
INSERT INTO table_50 (cartodb_id) VALUES ('1');
select cartodb.cdb_cartodbfytable('table_50');

CartoDBfication works, result:

 cartodb_id | the_geom | the_geom_webmercator | cartodb_id_0 | title 
------------+----------+----------------------+--------------+-------
          1 |          |                      | 1            | 
          2 |          |                      | 1            | 
(2 rows)

Scenario B:

CREATE TABLE table_51(
    cartodb_id    varchar,
    title         varchar(40)
);
INSERT INTO table_51 (cartodb_id) VALUES ('a');
INSERT INTO table_51 (cartodb_id) VALUES ('b');
select cartodb.cdb_cartodbfytable('table_51');

CartoDBfication fails:

ERROR:  CDB(_CDB_Rewrite_Table:22P02:invalid input syntax for integer: "a"

Both cases are out of the requirements (integer -- or casteable-- and unique), but both have different outcomes (silent rename VS crash 💥)

[iriberri]

The new commit makes failures consistent: everything will just crash now.

I've updated the tests too, so as soon as I fix the Rails part to give good feedback about the failure we're good.

[iriberri]

Tests passing :-) @rafatower could you give this a look again?

[rafatower]

👍

[iriberri]

Unused variable.

[iriberri]

My tests in staging are going on here: https://gist.github.com/iriberri/d839f6e67e05c6454a1d