Re: Foreign join pushdown vs EvalPlanQual - Mailing list pgsql-hackers

On 2015/08/12 7:21, Robert Haas wrote:
> On Fri, Aug 7, 2015 at 3:37 AM, Kouhei Kaigai <kaigai@ak.jp.nec.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)
*separately*.

* 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:

fdw-eval-plan-qual-0.1.patch
usermapping_matching.patch (in [3])
add_GetUserMappingById.patch (in [3])
foreign_join_v16_efujita.patch

(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
order.)

Comments welcome!

Best regards,
Etsuro Fujita

[1] http://www.postgresql.org/message-id/55B204A0.1080507@lab.ntt.co.jp
[2] http://www.postgresql.org/message-id/55B9F95F.5060506@lab.ntt.co.jp
[3]
http://www.postgresql.org/message-id/CAEZqfEe9KGy=1_waGh2rgZPg0o4pqgD+iauYaj8wTze+CYJUHg@mail.gmail.com