• Tom Lane's avatar
    Avoid crash during EvalPlanQual recheck of an inner indexscan. · 2e668c52
    Tom Lane authored
    Commit 09529a70 changed nodeIndexscan.c and nodeIndexonlyscan.c to
    postpone initialization of the indexscan proper until the first tuple
    fetch.  It overlooked the question of mark/restore behavior, which means
    that if some caller attempts to mark the scan before the first tuple fetch,
    you get a null pointer dereference.
    
    The only existing user of mark/restore is nodeMergejoin.c, which (somewhat
    accidentally) will never attempt to set a mark before the first inner tuple
    unless the inner child node is a Material node.  Hence the case can't arise
    normally, so it seems sufficient to document the assumption at both ends.
    However, during an EvalPlanQual recheck, ExecScanFetch doesn't call
    IndexNext but just returns the jammed-in test tuple.  Therefore, if we're
    doing a recheck in a plan tree with a mergejoin with inner indexscan,
    it's possible to reach ExecIndexMarkPos with iss_ScanDesc still null,
    as reported by Guo Xiang Tan in bug #15032.
    
    Really, when there's a test tuple supplied during an EPQ recheck, touching
    the index at all is the wrong thing: rather, the behavior of mark/restore
    ought to amount to saving and restoring the es_epqScanDone flag.  We can
    avoid finding a place to actually save the flag, for the moment, because
    given the assumption that no caller will set a mark before fetching a
    tuple, es_epqScanDone must always be set by the time we try to mark.
    So the actual behavior change required is just to not reach the index
    access if a test tuple is supplied.
    
    The set of plan node types that need to consider this issue are those
    that support EPQ test tuples (i.e., call ExecScan()) and also support
    mark/restore; which is to say, IndexScan, IndexOnlyScan, and perhaps
    CustomScan.  It's tempting to try to fix the problem in one place by
    teaching ExecMarkPos() itself about EPQ; but ExecMarkPos supports some
    plan types that aren't Scans, and also it seems risky to make assumptions
    about what a CustomScan wants to do here.  Also, the most likely future
    change here is to decide that we do need to support marks placed before
    the first tuple, which would require additional work in IndexScan and
    IndexOnlyScan in any case.  Hence, fix the EPQ issue in nodeIndexscan.c
    and nodeIndexonlyscan.c, accepting the small amount of code duplicated
    thereby, and leave it to CustomScan providers to fix this bug if they
    have it.
    
    Back-patch to v10 where commit 09529a70 came in.  In earlier branches,
    the index_markpos() call is a waste of cycles when EPQ is active, but
    no more than that, so it doesn't seem appropriate to back-patch further.
    
    Discussion: https://postgr.es/m/20180126074932.3098.97815@wrigleys.postgresql.org
    2e668c52
nodeIndexscan.c 52 KB