• Tom Lane's avatar
    Fix improper uses of canonicalize_qual(). · 4a4e2442
    Tom Lane authored
    One of the things canonicalize_qual() does is to remove constant-NULL
    subexpressions of top-level AND/OR clauses.  It does that on the assumption
    that what it's given is a top-level WHERE clause, so that NULL can be
    treated like FALSE.  Although this is documented down inside a subroutine
    of canonicalize_qual(), it wasn't mentioned in the documentation of that
    function itself, and some callers hadn't gotten that memo.
    
    Notably, commit d007a950 caused get_relation_constraints() to apply
    canonicalize_qual() to CHECK constraints.  That allowed constraint
    exclusion to misoptimize situations in which a CHECK constraint had a
    provably-NULL subclause, as seen in the regression test case added here,
    in which a child table that should be scanned is not.  (Although this
    thinko is ancient, the test case doesn't fail before 9.2, for reasons
    I've not bothered to track down in detail.  There may be related cases
    that do fail before that.)
    
    More recently, commit f0e44751 added an independent bug by applying
    canonicalize_qual() to index expressions, which is even sillier since
    those might not even be boolean.  If they are, though, I think this
    could lead to making incorrect index entries for affected index
    expressions in v10.  I haven't attempted to prove that though.
    
    To fix, add an "is_check" parameter to canonicalize_qual() to specify
    whether it should assume WHERE or CHECK semantics, and make it perform
    NULL-elimination accordingly.  Adjust the callers to apply the right
    semantics, or remove the call entirely in cases where it's not known
    that the expression has one or the other semantics.  I also removed
    the call in some cases involving partition expressions, where it should
    be a no-op because such expressions should be canonical already ...
    and was a no-op, independently of whether it could in principle have
    done something, because it was being handed the qual in implicit-AND
    format which isn't what it expects.  In HEAD, add an Assert to catch
    that type of mistake in future.
    
    This represents an API break for external callers of canonicalize_qual().
    While that's intentional in HEAD to make such callers think about which
    case applies to them, it seems like something we probably wouldn't be
    thanked for in released branches.  Hence, in released branches, the
    extra parameter is added to a new function canonicalize_qual_ext(),
    and canonicalize_qual() is a wrapper that retains its old behavior.
    
    Patch by me with suggestions from Dean Rasheed.  Back-patch to all
    supported branches.
    
    Discussion: https://postgr.es/m/24475.1520635069@sss.pgh.pa.us
    4a4e2442
prep.h 2.04 KB