Re: [PATCH] postgres_fdw: suppress explicit casts in text:text comparisons (was: column option to override foreign types)

"Dian M Fay" <dian(dot)m(dot)fay(at)gmail(dot)com> writes:
> [ 0001-Suppress-explicit-casts-of-text-constants-in-postgre.patch ]

I took a quick look at this. The restriction to type text seems like
very obviously a hack rather than something we actually want; wouldn't
it mean we fail to act in a large fraction of the cases where we'd
like to suppress the cast?

A second problem is that I don't think the business with a const_showtype
context field is safe at all. As you've implemented it here, it would
affect the entire RHS tree, including constants far down inside complex
expressions that have nothing to do with the top-level semantics.
(I didn't look closely, but I wonder if the regression failure you
mentioned is associated with that.)

I think that we only want to suppress the cast in cases where
(1) the constant is directly an operand of the operator we're
expecting the remote parser to use its same-type heuristic for, and
(2) the constant will be deparsed as a string literal. (If it's
deparsed as a number, boolean, etc, then it won't be initially
UNKNOWN, so that heuristic won't be applied.)

Now point 1 means that we don't really need to mess with keeping
state in the recursion context. If we've determined at the level
of the OpExpr that we can do this, including checking that the
RHS operand IsA(Const), then we can just invoke deparseConst() on
it directly instead of recursing via deparseExpr().

Meanwhile, I suspect that point 2 might be best checked within
deparseConst() itself, as that contains both the decision and the
mechanism about how the Const will be printed. So that suggests
that we should invent a new showtype code telling deparseConst()
to act this way, and then supply that code directly when we
invoke deparseConst directly from deparseOpExpr.

BTW, don't we also want to be able to optimize cases where the Const
is on the LHS rather than the RHS?

regards, tom lane