On Thu Mar 4, 2021 at 9:28 AM EST, Georgios Kokolatos wrote: > I am afraid I will have to :-1: this patch. Of course it is possible > that I am wrong, > so please correct me if you, or any other reviewers, think so. > > The problem that is intended to be solved, upon closer inspection > seems > to be a > conscious design decision rather than a problem. Even if I am wrong > there, I am > not certain that the proposed patch covers all the bases with respect > to > collations, > build-in types, shipability etc for simple expressions, and covers any > of more > complicated expressions all together.
Thanks for reviewing it! I see room for interpretation in the design here, although I have admittedly not been looking at it for very long. `CREATE/ALTER FOREIGN TABLE` take the user at their word about types, which only map 1:1 for a foreign Postgres server anyway. In make_tuple_from_result_row, incoming values start as strings until they're converted to their target types -- again, with no guarantee that those types match those on the remote server. The docs recommend types match exactly and note the sorts of things that can go wrong, but there's no enforcement; either what you've cooked up works or it doesn't. And in fact, declaring local text for a remote enum seems to work quite well.... right up until you try to reference it in the `WHERE` clause. Enum::text seems like a safe and potentially standardizable case for postgres_fdw. As implemented, the patch goes beyond that, but it's opt-in and the docs already warn about consequences. I haven't tested it across collations, but right now that seems like something to look into if the idea survives the next few messages. > I will add that creating a view on the remote server with type > flexibility that > you wish and then create foreign tables against the view, might > address > your > problem. A view would address the immediate issue of the types, but itself requires additional maintenance if/when the underlying table's schema changes (even `SELECT *` is expanded into the current column definitions at creation). I think it's better than copying the types, because it moves the extra work of keeping local and remote synchronized to a *table* modification as opposed to a *type* modification, in which latter case it's much easier to forget about dependents. But I'd prefer to avoid extra work anywhere!
