Re: Join push-down support for foreign tables

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>
Cc: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Join push-down support for foreign tables
Date: 2015-03-13 09:32:18
Message-ID: CAFjFpRcSeg7tA1W_DWors5L81s5KOJrpkPFUnuOyKJ+T2ReAKw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Hanada-san,
I noticed that the patch doesn't have any tests for testing FDW join in
postgres_fdw. While you are updating the patch, can you please add few
tests for the same. I will suggest adding tests for a combination of these
dimensions
1. Types of joins
2. Joins between multiple foreign and local tables together, to test
whether we are pushing maximum of join tree with mixed tables.
3. Join/Where conditions with un/safe-to-push expressions
4. Queries with sorting/aggregation on top of join to test working of
setref.
5. Joins between foreign tables on different foreign servers (to check that
those do not get accidently pushed down).

I have attached a file with some example queries on those lines.

On Tue, Mar 10, 2015 at 8:37 AM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:

> > Thanks for finding out what we oversight.
> > Here is still a problem because the new 'relids' field is not
> updated
> > on setrefs.c (scanrelid is incremented by rtoffset here).
> > It is easy to shift the bitmapset by rtoffset, however, I also
> would
> > like to see another approach.
> >
> >
> >
> > I just made it work for explain, but other parts still need work. Sorry
> about
> > that. If we follow INDEX_VAR, we should be able to get there.
> >
> I tried to modify your patch a bit as below:
> * add adjustment of bitmap fields on setrefs.c
> * add support on outfuncs.c and copyfuncs.c.
> * add bms_shift_members() in bitmapset.c
>
> I think it is a reasonable enhancement, however, it is not tested with
> real-life code, like postgres_fdw.
>
> Hanada-san, could you add a feature to print name of foreign-tables
> which are involved in remote queries, on postgresExplainForeignScan()?
> ForeignScan->fdw_relids bitmap and ExplainState->rtable_names will
> tell you the joined foreign tables replaced by the (pseudo) foreign-scan.
>
> Soon, I'll update the interface patch also.
>
> > My idea adds 'List *fdw_sub_paths' field in ForeignPath to inform
> > planner underlying foreign-scan paths (with scanrelid > 0).
> > The create_foreignscan_plan() will call create_plan_recurse() to
> > construct plan nodes based on the path nodes being attached.
> > Even though these foreign-scan nodes are not actually executed,
> > setrefs.c can update scanrelid in usual way and ExplainPreScanNode
> > does not need to take exceptional handling on Foreign/CustomScan
> > nodes.
> > In addition, it allows to keep information about underlying foreign
> > table scan, even if planner will need some other information in the
> > future version (not only relids).
> >
> > How about your thought?
> >
> >
> >
> > I am not sure about keeping planner nodes, which are not turned into
> execution
> > nodes. There's no precedence for that in current code. It could be risky.
> >
> Indeed, it is a fair enough opinion. At this moment, no other code makes
> plan
> node but shall not be executed actually.
> Please forget above idea.
>
> Thanks,
> --
> NEC OSS Promotion Center / PG-Strom Project
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
>
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Attachment Content-Type Size
pgfdw_join.sql application/octet-stream 2.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sreerama Manoj 2015-03-13 10:38:22 Fwd: Regarding pg_stat_statements
Previous Message Sreerama Manoj 2015-03-13 09:29:28 Regarding pg_stat_statements