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?