(Removing Adrien from the CC list, because messages to that address keep bouncing)
On Sun, 13 Jan 2019 at 00:31, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On 1/10/19 6:09 PM, Dean Rasheed wrote: > > > > In the previous discussion around UpdateStatisticsForTypeChange(), the > > consensus appeared to be that we should just unconditionally drop all > > extended statistics when ALTER TABLE changes the type of an included > > column (just as we do for per-column stats), since such a type change > > can rewrite the data in arbitrary ways, so there's no reason to assume > > that the old stats are still valid. I think it makes sense to extract > > that as a separate patch to be committed ahead of these ones, and I'd > > also argue for back-patching it. > > Wasn't the agreement to keep stats that don't include column values > (functional dependencies and ndistinct coefficients), and reset only > more complex stats? That's what happens in master and how it's extended > by the patch for MCV lists and histograms. > Ah OK, I misremembered the exact conclusion reached last time. In that case the logic in UpdateStatisticsForTypeChange() looks wrong: /* * If we can leave the statistics as it is, just do minimal cleanup * and we're done. */ if (!attribute_referenced && reset_stats) { ReleaseSysCache(oldtup); return; } That should be "|| !reset_stats", or have more parentheses. In fact, I think that computing attribute_referenced is unnecessary because the dependency information includes the columns that the stats are for and ATExecAlterColumnType() uses that, so attribute_referenced will always be true. Regards, Dean