CartoDB / cartodb-postgresql

PostgreSQL extension for CartoDB
BSD 3-Clause "New" or "Revised" License
111 stars 53 forks source link

Improve Ghost Tables functions #360

Closed gonzaloriestra closed 5 years ago

gonzaloriestra commented 5 years ago

AFAIK, this should be equivalent, but it seems to fix the random failures in cartodb CI.

Algunenano commented 5 years ago

I'm not sure why this is making a difference, specially since the documentation recommends not to use a explicit name:

Tip
It is often preferable to use unique index inference rather than naming a constraint directly using ON CONFLICT ON CONSTRAINT constraint_name. Inference will continue to work correctly when the underlying index is replaced by another more or less equivalent index in an overlapping way, for example when using CREATE UNIQUE INDEX ... CONCURRENTLY before dropping the index being replace

Note that since you are using the table primary key as constraint (cdb_ddl_execution_pkey) I don't think this applies to us since you aren't going to reindex it, right? In any case, being explicit should be fine.

There is a couple of things I find odd after looking at the whole trigger process:

gonzaloriestra commented 5 years ago

Regarding the fix, I had seen the tip from the documentation, but as you say, I don't think is a problem here.

It also says this: conflict_target can perform unique index inference. [...] If an attempt at inference is unsuccessful, an error is raised.

I still don't know why the inference could fail here (and only sometimes), but at least we know that it can happen, that's why I tried to use the explicit constraint instead of the inference.

gonzaloriestra commented 5 years ago

About your suggestions, good ones! I'm going to add both in this PR.

@ethervoid @javitonino I'll add CREATE/ALTER/DROP FOREIGN TABLE to the ghost tables events, ok?