On Sat, 22 Dec 2018 at 07:10, Tom Lane <t...@sss.pgh.pa.us> wrote:

> BTW, with respect to this bit in 0001:
>
> @@ -1795,6 +1847,15 @@ nulltestsel(PlannerInfo *root, NullTestType
> nulltesttype, Node *arg,
>                  return (Selectivity) 0; /* keep compiler quiet */
>          }
>      }
> +    else if (vardata.var && IsA(vardata.var, Var) &&
> +             ((Var *) vardata.var)->varattno ==
> SelfItemPointerAttributeNumber)
> +    {
> +        /*
> +         * There are no stats for system columns, but we know CTID is
> never
> +         * NULL.
> +         */
> +        selec = (nulltesttype == IS_NULL) ? 0.0 : 1.0;
> +    }
>      else
>      {
>          /*
>
> I'm not entirely sure why you're bothering; surely nulltestsel is
> unrelated to what this patch is about?  And would anybody really
> write "WHERE ctid IS NULL"?
>

I found that it made a difference with selectivity of range comparisons,
because clauselist_selectivity tries to correct for it (clausesel.c:274):

    s2 = rqlist->hibound + rqlist->lobound - 1.0

    /* Adjust for double-exclusion of NULLs */
    s2 += nulltestsel(root, IS_NULL, rqlist->var,
                      varRelid, jointype, sjinfo);

It was adding DEFAULT_UNK_SEL = 0.005 to the selectivity, which (while not
major) did make the selectivity less accurate.

However, if we do think it's worth adding code to cover this case,
> I wouldn't make it specific to CTID.  *All* system columns can be
> assumed not null, see heap_getsysattr().
>
I guess we could have a standalone patch to add this for all system columns?

Reply via email to