Re: ORDER BY pushdowns seem broken in postgres_fdw - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: ORDER BY pushdowns seem broken in postgres_fdw
Date
Msg-id CALNJ-vRRmQde3osXr40+k1vWC0K=cfr8PaNzd0iu87Di64JLsA@mail.gmail.com
Whole thread Raw
In response to Re: ORDER BY pushdowns seem broken in postgres_fdw  (Ronan Dunklau <ronan.dunklau@aiven.io>)
List pgsql-hackers


On Mon, Sep 6, 2021 at 2:41 AM Ronan Dunklau <ronan.dunklau@aiven.io> wrote:
Le lundi 6 septembre 2021, 11:25:39 CEST Zhihong Yu a écrit :
> On Mon, Sep 6, 2021 at 1:17 AM Ronan Dunklau <ronan@dunklau.fr> wrote:
> > Le vendredi 3 septembre 2021, 22:54:25 CEST David Zhang a écrit :
> > > The following review has been posted through the commitfest application:
> > > make installcheck-world:  tested, failed
> > > Implements feature:       tested, passed
> > > Spec compliant:           not tested
> > > Documentation:            not tested
> > >
> > > Applied the v6 patch to master branch and ran regression test for
> >
> > contrib,
> >
> > > the result was "All tests successful."
> >
> > What kind of error did you get running make installcheck-world ? If it
> > passed
> > the make check for contrib, I can't see why it would fail running make
> > installcheck-world.
> >
> > In any case, I just checked and running make installcheck-world doesn't
> > produce any error.
> >
> > Since HEAD had moved a bit since the last version, I rebased the patch,
> > resulting in the attached v7.
> >
> > Best regards,
> >
> > --
> > Ronan Dunklau
>
> Hi,
> bq. a pushed-down order by could return wrong results.
>
> Can you briefly summarize the approach for fixing the bug in the
> description ?

Done, let me know what you think about it.

>
> + * Returns true if it's safe to push down a sort as described by 'pathkey'
> to
> + * the foreign server
> + */
> +bool
> +is_foreign_pathkey(PlannerInfo *root,
>
> It would be better to name the method which reflects whether pushdown is
> safe. e.g. is_pathkey_safe_for_pushdown.

The convention used here is the same one as in is_foreign_expr and
is_foreign_param, which are also related to pushdown-safety.

--
Ronan Dunklau
Hi,
w.r.t. description:
bq. original operator associated to the pathkey

 associated to -> associated with

w.r.t. method name, it is fine to use the current name, considering the functions it calls don't have pushdown in their names.

Cheers

pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Avoid stuck of pbgench due to skipped transactions
Next
From: Fujii Masao
Date:
Subject: Re: corruption of WAL page header is never reported