Re: [PATCH] postgres-fdw: column option to override foreign types

"Dian M Fay" <dian(dot)m(dot)fay(at)gmail(dot)com> writes:
> On Thu Mar 4, 2021 at 9:28 AM EST, Georgios Kokolatos wrote:
>> I am afraid I will have to :-1: this patch.

> 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.

Right.

> 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 data conversion itself provides a little bit of security --- for
instance, converting 'foobar' to int or timestamp will fail. It's
not bulletproof, but on the other hand there are indeed situations
where you don't want to declare the column locally with exactly the
type the remote server is using, so trying to be bulletproof would
be counterproductive.

I am not, however, any more impressed than the other respondents with
the solution you've proposed. For one thing, this can only help if
the local type is known to the remote server, which seems to eliminate
fifty per cent of the use-cases for intentional differences in type.
(That is, isn't it equally as plausible that the local type is an
enum you didn't bother making on the remote side?) But a bigger issue
is that shipping
WHERE foreigncol::text = 'one'::text
to the remote server is not a nice solution even if it works. It will,
for example, defeat use of a normal index on foreigncol. It'd likely
be just as inefficient for remote joins.

What'd be better, if we could do it, is to ship the clause in
the form
WHERE foreigncol = 'one'
that is, instead of plastering a cast on the Var, try to not put
any explicit cast on the constant. That fixes your original use
case better than what you've proposed, and I think it might be
possible to do it unconditionally instead of needing a hacky
column property to enable it. The reason this could be okay
is that it seems reasonable for postgres_fdw to rely on the
core parser's heuristic that an unknown-type literal is the
same type as what it's being compared to. So, if we are trying
to deparse something of the form "foreigncol operator constant",
and the foreigncol and constant are of the same type locally,
we could leave off the cast on the constant. (There might need
to be some restrictions about the operator taking those types
natively with no cast, not sure; also this doesn't apply to
constants that are going to be printed as non-string literals.)

Slipping this heuristic into the code structure of deparse.c
might be rather messy, though. I've not looked at just how
to implement it.

regards, tom lane