Hello, This sounded like an interesting addition to postgresql. I gave some time to it today to review, here are few comments,
On Wed, 25 Mar 2020 at 14:28, Konstantin Knizhnik <k.knizh...@postgrespro.ru> wrote: > > > > On 25.03.2020 16:00, David Steele wrote: > > On 3/25/20 6:57 AM, Konstantin Knizhnik wrote: > >> > >> > >> On 24.03.2020 20:12, David Steele wrote: > >>> On 12/24/19 3:15 AM, Konstantin Knizhnik wrote: > >>>> New version of patch implicitly adding multicolumn statistic in > >>>> auto_explain extension and using it in optimizer for more precise > >>>> estimation of join selectivity. > >>>> This patch fixes race condition while adding statistics and > >>>> restricts generated statistic name to fit in 64 bytes (NameData). > >>> > >>> This patch no longer applies: > >>> https://commitfest.postgresql.org/27/2386/ > >>> > >>> The CF entry has been updated to Waiting on Autho > >> > >> Rebased patch is attached. > > > > The patch applies now but there are error on Windows and Linux: > > https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.85481 > > > > https://travis-ci.org/postgresql-cfbot/postgresql/builds/666729979 > > > > Regards, > > Sorry, yet another patch is attached. > > -- > Konstantin Knizhnik > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company > +static void +AddMultiColumnStatisticsForNode(PlanState *planstate, ExplainState *es); + This doesn't look like the right place for it, you might want to declare it with other functions in the starting of the file. Also, there is no description about any of the functions here, wouldn’t hurt having some more comments there. A few of more questions that cross my mind at this point, - have you tried measuring the extra cost we have to pay for this mores statistics , and also compare it with the benefit it gives in terms of accuracy. - I would also be interested in understanding if there are cases when adding this extra step doesn’t help and have you excluded them already or if some of them are easily identifiable at this stage...? - is there any limit on the number of columns for which this will work, or should there be any such limit...? -- Regards, Rafia Sabih
