Re: BUG #18467: postgres_fdw (deparser) ignores LimitOption

On Fri, 17 May 2024 at 00:58, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
>> On 2024-May-16, Japin Li wrote:
>>> I find that the FETCH FIRST ... ROWS WITH TIES does not support type casting.
>
>> Why do you need this? The standard says
>
>> <fetch first clause> ::= FETCH { FIRST | NEXT } [ <fetch first quantity> ] { ROW | ROWS } { ONLY | WITH TIES }
>> <fetch first quantity> ::= <fetch first row count> | <fetch first percentage>
>> <offset row count> ::= <simple value specification>
>> <fetch first row count> ::= <simple value specification>
>> which doesn't seem to leave room for a cast.
>
> Exactly. "LIMIT 42::int8" is accepted, but the equivalent
> with FETCH is not, so that there's a hazard of the remote
> rejecting the query depending on how deparseExpr chooses
> to print the limit expression. Ordinarily it hasn't the
> slightest hesitation about affixing casts to constants,
> so I suspect there's a live problem there.
>
> I think that this approach to a fix might be the wrong thing
> altogether, and that it might be better to refuse to send WITH_TIES
> clauses to the remote at all. There are two things that are scaring
> me about that:
>
> 1. The remote might have a different idea of equality than we do,
> leading to different results. Admittedly that's a little bit
> far-fetched, but with things like nondeterministic collations floating
> around, it's by no means impossible.
>
> 2. If the remote is older than v13 the query will fail entirely
> for lack of support for WITH TIES.
>

I didn't take this into consideration previously.

> At the very least we need a remote-version check before deciding
> that it'll be OK to ship such a clause. But if there are other
> gotchas, as it seems there are, let's just cut our losses and
> not do it.
>

Try to disable the push-down FETCH FIRST clause as you suggested.

--
Regards,
Japin Li