• Tom Lane's avatar
    Code review for foreign/custom join pushdown patch. · 1a8a4e5c
    Tom Lane authored
    Commit e7cb7ee1 included some design
    decisions that seem pretty questionable to me, and there was quite a lot
    of stuff not to like about the documentation and comments.  Clean up
    as follows:
    
    * Consider foreign joins only between foreign tables on the same server,
    rather than between any two foreign tables with the same underlying FDW
    handler function.  In most if not all cases, the FDW would simply have had
    to apply the same-server restriction itself (far more expensively, both for
    lack of caching and because it would be repeated for each combination of
    input sub-joins), or else risk nasty bugs.  Anyone who's really intent on
    doing something outside this restriction can always use the
    set_join_pathlist_hook.
    
    * Rename fdw_ps_tlist/custom_ps_tlist to fdw_scan_tlist/custom_scan_tlist
    to better reflect what they're for, and allow these custom scan tlists
    to be used even for base relations.
    
    * Change make_foreignscan() API to include passing the fdw_scan_tlist
    value, since the FDW is required to set that.  Backwards compatibility
    doesn't seem like an adequate reason to expect FDWs to set it in some
    ad-hoc extra step, and anyway existing FDWs can just pass NIL.
    
    * Change the API of path-generating subroutines of add_paths_to_joinrel,
    and in particular that of GetForeignJoinPaths and set_join_pathlist_hook,
    so that various less-used parameters are passed in a struct rather than
    as separate parameter-list entries.  The objective here is to reduce the
    probability that future additions to those parameter lists will result in
    source-level API breaks for users of these hooks.  It's possible that this
    is even a small win for the core code, since most CPU architectures can't
    pass more than half a dozen parameters efficiently anyway.  I kept root,
    joinrel, outerrel, innerrel, and jointype as separate parameters to reduce
    code churn in joinpath.c --- in particular, putting jointype into the
    struct would have been problematic because of the subroutines' habit of
    changing their local copies of that variable.
    
    * Avoid ad-hocery in ExecAssignScanProjectionInfo.  It was probably all
    right for it to know about IndexOnlyScan, but if the list is to grow
    we should refactor the knowledge out to the callers.
    
    * Restore nodeForeignscan.c's previous use of the relcache to avoid
    extra GetFdwRoutine lookups for base-relation scans.
    
    * Lots of cleanup of documentation and missed comments.  Re-order some
    code additions into more logical places.
    1a8a4e5c
nodeIndexonlyscan.c 17 KB