• Tom Lane's avatar
    Refactor planning of projection steps that don't need a Result plan node. · 8b9d323c
    Tom Lane authored
    The original upper-planner-pathification design (commit 3fc6e2d7)
    assumed that we could always determine during Path formation whether or not
    we would need a Result plan node to perform projection of a targetlist.
    That turns out not to work very well, though, because createplan.c still
    has some responsibilities for choosing the specific target list associated
    with sorting/grouping nodes (in particular it might choose to add resjunk
    columns for sorting).  We might not ever refactor that --- doing so would
    push more work into Path formation, which isn't attractive --- and we
    certainly won't do so for 9.6.  So, while create_projection_path and
    apply_projection_to_path can tell for sure what will happen if the subpath
    is projection-capable, they can't tell for sure when it isn't.  This is at
    least a latent bug in apply_projection_to_path, which might think it can
    apply a target to a non-projecting node when the node will end up computing
    something different.
    
    Also, I'd tied the creation of a ProjectionPath node to whether or not a
    Result is needed, but it turns out that we sometimes need a ProjectionPath
    node anyway to avoid modifying a possibly-shared subpath node.  Callers had
    to use create_projection_path for such cases, and we added code to them
    that knew about the potential omission of a Result node and attempted to
    adjust the cost estimates for that.  That was uncertainly correct and
    definitely ugly/unmaintainable.
    
    To fix, have create_projection_path explicitly check whether a Result
    is needed and adjust its cost estimate accordingly, though it creates
    a ProjectionPath in either case.  apply_projection_to_path is now mostly
    just an optimized version that can avoid creating an extra Path node when
    the input is known to not be shared with any other live path.  (There
    is one case that create_projection_path doesn't handle, which is pushing
    parallel-safe expressions below a Gather node.  We could make it do that
    by duplicating the GatherPath, but there seems no need as yet.)
    
    create_projection_plan still has to recheck the tlist-match condition,
    which means that if the matching situation does get changed by createplan.c
    then we'll have made a slightly incorrect cost estimate.  But there seems
    no help for that in the near term, and I doubt it occurs often enough,
    let alone would change planning decisions often enough, to be worth
    stressing about.
    
    I added a "dummypp" field to ProjectionPath to track whether
    create_projection_path thinks a Result is needed.  This is not really
    necessary as-committed because create_projection_plan doesn't look at the
    flag; but it seems like a good idea to remember what we thought when
    forming the cost estimate, if only for debugging purposes.
    
    In passing, get rid of the target_parallel parameter added to
    apply_projection_to_path by commit 54f5c515.  I don't think that's a good
    idea because it involves callers in what should be an internal decision,
    and opens us up to missing optimization opportunities if callers think they
    don't need to provide a valid flag, as most don't.  For the moment, this
    just costs us an extra has_parallel_hazard call when planning a Gather.
    If that starts to look expensive, I think a better solution would be to
    teach PathTarget to carry/cache knowledge of parallel-safety of its
    contents.
    8b9d323c
outfuncs.c 88.9 KB