Re: Foreign join pushdown vs EvalPlanQual

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, 花田茂 <shigeru(dot)hanada(at)gmail(dot)com>
Subject: Re: Foreign join pushdown vs EvalPlanQual
Date: 2015-08-12 11:25:57
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2015/08/12 7:21, Robert Haas wrote:
> On Fri, Aug 7, 2015 at 3:37 AM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>>> I could have a discussion with Fujita-san about this topic.
>> Also, let me share with the discussion towards entire solution.
>> The primitive reason of this problem is, Scan node with scanrelid==0
>> represents a relation join that can involve multiple relations, thus,
>> its TupleDesc of the records will not fit base relations, however,
>> ExecScanFetch() was not updated when scanrelid==0 gets supported.
>> FDW/CSP on behalf of the Scan node with scanrelid==0 are responsible
>> to generate records according to the fdw_/custom_scan_tlist that
>> reflects the definition of relation join, and only FDW/CSP know how
>> to combine these base relations.
>> In addition, host-side expressions (like Plan->qual) are initialized
>> to reference the records generated by FDW/CSP, so the least invasive
>> approach is to allow FDW/CSP to have own logic to recheck, I think.
>> Below is the structure of ExecScanFetch().
>> ExecScanFetch(ScanState *node,
>> ExecScanAccessMtd accessMtd,
>> ExecScanRecheckMtd recheckMtd)
>> {
>> EState *estate = node->ps.state;
>> if (estate->es_epqTuple != NULL)
>> {
>> /*
>> * We are inside an EvalPlanQual recheck. Return the test tuple if
>> * one is available, after rechecking any access-method-specific
>> * conditions.
>> */
>> Index scanrelid = ((Scan *) node->ps.plan)->scanrelid;
>> Assert(scanrelid > 0);
>> if (estate->es_epqTupleSet[scanrelid - 1])
>> {
>> TupleTableSlot *slot = node->ss_ScanTupleSlot;
>> :
>> return slot;
>> }
>> }
>> return (*accessMtd) (node);
>> }
>> When we are inside of EPQ, it fetches a tuple in es_epqTuple[] array and
>> checks its visibility (ForeignRecheck() always say 'yep, it is visible'),
>> then ExecScan() applies its qualifiers by ExecQual().
>> So, as long as FDW/CSP can return a record that satisfies the TupleDesc
>> of this relation, made by the tuples in es_epqTuple[] array, rest of the
>> code paths are common.
>> I have an idea to solve the problem.
>> It adds recheckMtd() call if scanrelid==0 just before the assertion above,
>> and add a callback of FDW on ForeignRecheck().
>> The role of this new callback is to set up the supplied TupleTableSlot
>> and check its visibility, but does not define how to do this.
>> It is arbitrarily by FDW driver, like invocation of alternative plan
>> consists of only built-in logic.
>> Invocation of alternative plan is one of the most feasible way to
>> implement EPQ logic on FDW, so I think FDW also needs a mechanism
>> that takes child path-nodes like custom_paths in CustomPath node.
>> Once a valid path node is linked to this list, createplan.c transform
>> them to relevant plan node, then FDW can initialize and invoke this
>> plan node during execution, like ForeignRecheck().
>> This design can solve another problem Fujita-san has also mentioned.
>> If scan qualifier is pushed-down to the remote query and its expression
>> node is saved in the private area of ForeignScan, the callback on
>> ForeignRecheck() can evaluate the qualifier by itself. (Note that only
>> FDW driver can know where and how expression node being pushed-down
>> is saved in the private area.)
>> In the summary, the following three enhancements are a straightforward
>> way to fix up the problem he reported.
>> 1. Add a special path to call recheckMtd in ExecScanFetch if scanrelid==0
>> 2. Add a callback of FDW in ForeignRecheck() - to construct a record
>> according to the fdw_scan_tlist definition and to evaluate its
>> visibility, or to evaluate qualifier pushed-down if base relation.
>> 3. Add List *fdw_paths in ForeignPath like custom_paths of CustomPaths,
>> to construct plan nodes for EPQ evaluation.

> I'm not an expert in this area, but this plan does not seem unreasonable to me.

IIRC the discussion with KaiGai-san, I think that that would work. I
think that that would be more suitable for CSPs, though. Correct me if
I'm wrong, KaiGai-san. In either case, I'm not sure that the idea of
transferring both processing to a single callback routine hooked in
ForeignRecheck is a good idea: (a) check to see if the test tuple for
each component foreign table satisfies the remote qual condition and (b)
check to see if those tuples satisfy the remote join condition. I think
that that would be too complicated, probably making the callback routine
bug-prone. So, I'd still propose that *the core* processes (a) and (b)

* As for (a), the core checks the remote qual condition as in [1].

* As for (b), the core executes an alternative subplan locally if inside
an EPQ recheck. The subplan is created as described in [2].

Attached is a WIP patch for that against 9.5
(fdw-eval-plan-qual-0.1.patch), which includes an updated version of the
patch in [1]. I haven't done anything about custom joins yet. Also, I
left the join pushdown API as-is. But I still think that it would be
better that we hook that API in standard_join_search. So, I plan to
modify the patch so in the next version.

For tests, I did a very basic update of the latest postgres_fdw patch in
[3] and attach that (foreign_join_v16_efujita.patch). You can apply the
patches in the following order:

usermapping_matching.patch (in [3])
add_GetUserMappingById.patch (in [3])

(Note that you cannot do tests of [1]. For that, apply
fdw-eval-plan-qual-0.1.patch and the postgres_fdw patch in [1] in this

Comments welcome!

Best regards,
Etsuro Fujita


Attachment Content-Type Size
fdw-eval-plan-qual-0.1.patch text/x-patch 15.5 KB
foreign_join_v16_efujita.patch text/x-patch 106.3 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2015-08-12 12:16:09 Re: Warnings around booleans
Previous Message Andres Freund 2015-08-12 10:41:40 Re: Warnings around booleans