From: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
---|---|
To: | Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io> |
Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, David Zhang <david(dot)zhang(at)highgo(dot)ca> |
Subject: | Re: ORDER BY pushdowns seem broken in postgres_fdw |
Date: | 2021-09-06 16:46:37 |
Message-ID: | CALNJ-vRRmQde3osXr40+k1vWC0K=cfr8PaNzd0iu87Di64JLsA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Sep 6, 2021 at 2:41 AM Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)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(at)dunklau(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2021-09-06 17:02:38 | Re: corruption of WAL page header is never reported |
Previous Message | Fujii Masao | 2021-09-06 16:10:44 | Re: Avoid stuck of pbgench due to skipped transactions |