| From: | Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io> | 
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org | 
| Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, David Zhang <david(dot)zhang(at)highgo(dot)ca>, Zhihong Yu <zyu(at)yugabyte(dot)com> | 
| Subject: | Re: ORDER BY pushdowns seem broken in postgres_fdw | 
| Date: | 2021-09-06 09:41:26 | 
| Message-ID: | 2383910.KNGdDaisV1@aivenronan | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
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
| Attachment | Content-Type | Size | 
|---|---|---|
| v8-0001-Fix-orderby-handling-in-postgres_fdw.patch | text/x-patch | 17.6 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2021-09-06 09:54:59 | Re: Diagnostic comment in LogicalIncreaseXminForSlot | 
| Previous Message | Zhihong Yu | 2021-09-06 09:25:39 | Re: ORDER BY pushdowns seem broken in postgres_fdw |