On Mon, Mar 9, 2015 at 5:46 AM, Kouhei Kaigai <kai...@ak.jp.nec.com> wrote:

> Hi Ashutosh,
>
> 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.


>
> 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.


> --
> NEC OSS Promotion Center / PG-Strom Project
> KaiGai Kohei <kai...@ak.jp.nec.com>
>
>
> > -----Original Message-----
> > From: pgsql-hackers-ow...@postgresql.org
> > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Ashutosh Bapat
> > Sent: Friday, March 06, 2015 7:26 PM
> > To: Kaigai Kouhei(海外 浩平)
> > Cc: Shigeru Hanada; Robert Haas; PostgreSQL-development
> > Subject: Re: [HACKERS] Join push-down support for foreign tables
> >
> > Hi Kaigai-san, Hanada-san,
> >
> > Attached please find a patch to print the column names prefixed by the
> relation
> > names. I haven't tested the patch fully. The same changes will be needed
> for
> > CustomPlan node specific code.
> >
> > Now I am able to make sense out of the Output information
> >
> > postgres=# explain verbose select * from ft1 join ft2 using (val);
> >
> > QUERY PLAN
> >
> >
> ----------------------------------------------------------------------------
> >
> ---------------------------------------------------------------------------
> > -----------------------
> >  Foreign Scan  (cost=100.00..125.60 rows=2560 width=12)
> >    Output: ft1.val, ft1.val2, ft2.val2
> >    Remote SQL: SELECT r.a_0, r.a_1, l.a_1 FROM (SELECT val, val2 FROM
> public.lt)
> > l (a_0, a_1) INNER JOIN (SELECT val, val2 FROM public.lt) r (a_0, a_1)
> >   ON ((r.a_0 = l.a_0))
> > (3 rows)
> >
> >
> >
> > On Fri, Mar 6, 2015 at 6:41 AM, Kouhei Kaigai <kai...@ak.jp.nec.com>
> wrote:
> >
> >
> >       > Actually val and val2 come from public.lt in "r" side, but as
> you say
> >       > it's too difficult to know that from EXPLAIN output.  Do you
> have any
> >       > idea to make the "Output" item more readable?
> >       >
> >       A fundamental reason why we need to have symbolic aliases here is
> that
> >       postgres_fdw has remote query in cstring form. It makes
> implementation
> >       complicated to deconstruct/construct a query that is once
> constructed
> >       on the underlying foreign-path level.
> >       If ForeignScan keeps items to construct remote query in expression
> node
> >       form (and construction of remote query is delayed to beginning of
> the
> >       executor, probably), we will be able to construct more human
> readable
> >       remote query.
> >
> >       However, I don't recommend to work on this great refactoring stuff
> >       within the scope of join push-down support project.
> >
> >       Thanks,
> >       --
> >       NEC OSS Promotion Center / PG-Strom Project
> >       KaiGai Kohei <kai...@ak.jp.nec.com>
> >
> >
> >       > -----Original Message-----
> >       > From: pgsql-hackers-ow...@postgresql.org
> >       > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Shigeru
> > Hanada
> >       > Sent: Thursday, March 05, 2015 10:00 PM
> >       > To: Ashutosh Bapat
> >       > Cc: Kaigai Kouhei(海外 浩平); Robert Haas; PostgreSQL-development
> >       > Subject: Re: [HACKERS] Join push-down support for foreign tables
> >       >
> >
> >       > Hi Ashutosh, thanks for the review.
> >       >
> >       > 2015-03-04 19:17 GMT+09:00 Ashutosh Bapat
> > <ashutosh.ba...@enterprisedb.com>:
> >       > > In create_foreignscan_path() we have lines like -
> >       > > 1587     pathnode->path.param_info =
> > get_baserel_parampathinfo(root, rel,
> >       > > 1588
> >       > > required_outer);
> >       > > Now, that the same function is being used for creating foreign
> scan
> > paths
> >       > > for joins, we should be calling get_joinrel_parampathinfo() on
> a join
> > rel
> >       > > and get_baserel_parampathinfo() on base rel.
> >       >
> >       > Got it.  Please let me check the difference.
> >       >
> >       > >
> >       > > The patch seems to handle all the restriction clauses in the
> same
> > way. There
> >       > > are two kinds of restriction clauses - a. join quals
> (specified using
> > ON
> >       > > clause; optimizer might move them to the other class if that
> doesn't
> > affect
> >       > > correctness) and b. quals on join relation (specified in the
> WHERE
> > clause,
> >       > > optimizer might move them to the other class if that doesn't
> affect
> >       > > correctness). The quals in "a" are applied while the join is
> being
> > computed
> >       > > whereas those in "b" are applied after the join is computed.
> For
> > example,
> >       > > postgres=# select * from lt;
> >       > >  val | val2
> >       > > -----+------
> >       > >    1 |    2
> >       > >    1 |    3
> >       > > (2 rows)
> >       > >
> >       > > postgres=# select * from lt2;
> >       > >  val | val2
> >       > > -----+------
> >       > >    1 |    2
> >       > > (1 row)
> >       > >
> >       > > postgres=# select * from lt left join lt2 on (lt.val2 =
> lt2.val2);
> >       > >  val | val2 | val | val2
> >       > > -----+------+-----+------
> >       > >    1 |    2 |   1 |    2
> >       > >    1 |    3 |     |
> >       > > (2 rows)
> >       > >
> >       > > postgres=# select * from lt left join lt2 on (true) where
> (lt.val2
> > =
> >       > > lt2.val2);
> >       > >  val | val2 | val | val2
> >       > > -----+------+-----+------
> >       > >    1 |    2 |   1 |    2
> >       > > (1 row)
> >       > >
> >       > > The difference between these two kinds is evident in case of
> outer
> > joins,
> >       > > for inner join optimizer puts all of them in class "b". The
> remote
> > query
> >       > > sent to the foreign server has all those in ON clause.
> Consider foreign
> >       > > tables ft1 and ft2 pointing to local tables on the same server.
> >       > > postgres=# \d ft1
> >       > >          Foreign table "public.ft1"
> >       > >  Column |  Type   | Modifiers | FDW Options
> >       > > --------+---------+-----------+-------------
> >       > >  val    | integer |           |
> >       > >  val2   | integer |           |
> >       > > Server: loopback
> >       > > FDW Options: (table_name 'lt')
> >       > >
> >       > > postgres=# \d ft2
> >       > >          Foreign table "public.ft2"
> >       > >  Column |  Type   | Modifiers | FDW Options
> >       > > --------+---------+-----------+-------------
> >       > >  val    | integer |           |
> >       > >  val2   | integer |           |
> >       > > Server: loopback
> >       > > FDW Options: (table_name 'lt2')
> >       > >
> >       > > postgres=# explain verbose select * from ft1 left join ft2 on
> (ft1.val2
> > =
> >       > > ft2.val2) where ft1.val + ft2.val > ft1.val2 or ft2.val is
> null;
> >       > >
> >       > > QUERY PLAN
> >       > >
> >       > >
> >       >
> >
> ----------------------------------------------------------------------------
> >       >
> >
> ---------------------------------------------------------------------------
> >       > >
> >       >
> >
> ----------------------------------------------------------------------------
> >       > --------
> >       > >  Foreign Scan  (cost=100.00..125.60 rows=2560 width=16)
> >       > >    Output: val, val2, val, val2
> >       > >    Remote SQL: SELECT r.a_0, r.a_1, l.a_0, l.a_1 FROM (SELECT
> val,
> > val2 FROM
> >       > > public.lt2) l (a_0, a_1) RIGHT JOIN (SELECT val, val2 FROM
> public.lt)
> > r (a
> >       > > _0, a_1)  ON ((((r.a_0 + l.a_0) > r.a_1) OR (l.a_0 IS NULL)))
> AND
> > ((r.a_1 =
> >       > > l.a_1))
> >       > > (3 rows)
> >       > >
> >       > > The result is then wrong
> >       > > postgres=# select * from ft1 left join ft2 on (ft1.val2 =
> ft2.val2)
> > where
> >       > > ft1.val + ft2.val > ft1.val2 or ft2.val is null;
> >       > >  val | val2 | val | val2
> >       > > -----+------+-----+------
> >       > >    1 |    2 |     |
> >       > >    1 |    3 |     |
> >       > > (2 rows)
> >       > >
> >       > > which should match the result obtained by substituting local
> tables
> > for
> >       > > foreign ones
> >       > > postgres=# select * from lt left join lt2 on (lt.val2 =
> lt2.val2)
> > where
> >       > > lt.val + lt2.val > lt.val2 or lt2.val is null;
> >       > >  val | val2 | val | val2
> >       > > -----+------+-----+------
> >       > >    1 |    3 |     |
> >       > > (1 row)
> >       > >
> >       > > Once we start distinguishing the two kinds of quals, there is
> some
> >       > > optimization possible. For pushing down a join it's essential
> that
> > all the
> >       > > quals in "a" are safe to be pushed down. But a join can be
> pushed
> > down, even
> >       > > if quals in "a" are not safe to be pushed down. But more
> clauses one
> > pushed
> >       > > down to foreign server, lesser are the rows fetched from the
> foreign
> > server.
> >       > > In postgresGetForeignJoinPath, instead of checking all the
> > restriction
> >       > > clauses to be safe to be pushed down, we need to check only
> those
> > which are
> >       > > join quals (class "a").
> >       >
> >       > The argument restrictlist of GetForeignJoinPaths contains both
> >       > conditions mixed, so I added extract_actual_join_clauses() to
> separate
> >       > it into two lists, join_quals and other clauses.  This is
> similar to
> >       > what create_nestloop_plan and siblings do.
> >       >
> >       >
> >       > >
> >       > > Following EXPLAIN output seems to be confusing
> >       > > ft1 and ft2 both are pointing to same lt on a foreign server.
> >       > > postgres=# explain verbose select ft1.val + ft1.val2 from ft1,
> ft2
> > where
> >       > > ft1.val + ft1.val2 = ft2.val;
> >       > >
> >       > > QUERY PLAN
> >       > >
> >       > >
> >       >
> >
> ----------------------------------------------------------------------------
> >       >
> >
> ---------------------------------------------------------------------------
> >       > > --------------------------
> >       > >  Foreign Scan  (cost=100.00..132.00 rows=2560 width=8)
> >       > >    Output: (val + val2)
> >       > >    Remote SQL: SELECT r.a_0, r.a_1 FROM (SELECT val, NULL FROM
> > public.lt) l
> >       > > (a_0, a_1) INNER JOIN (SELECT val, val2 FROM public.lt) r
> (a_0, a_1)
> > ON ((
> >       > > (r.a_0 + r.a_1) = l.a_0))
> >       > >
> >       > > Output just specified val + val2, it doesn't tell, where those
> val
> > and val2
> >       > > come from, neither it's evident from the rest of the context.
> >       > >
> >       >
> >       > Actually val and val2 come from public.lt in "r" side, but as
> you say
> >       > it's too difficult to know that from EXPLAIN output.  Do you
> have any
> >       > idea to make the "Output" item more readable?
> >       >
> >       > --
> >       > Shigeru HANADA
> >       >
> >       >
> >
> >       > --
> >       > Sent via pgsql-hackers mailing list (
> pgsql-hackers@postgresql.org)
> >       > To make changes to your subscription:
> >       > http://www.postgresql.org/mailpref/pgsql-hackers
> >
> >
> >
> >
> >
> > --
> >
> > Best Wishes,
> > Ashutosh Bapat
> > EnterpriseDB Corporation
> > The Postgres Database Company
>
>


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

Reply via email to