Subject: Re: [HACKERS] PATCH: multivariate histograms and MCV lists
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>,
 David Rowley <david.rowley@2ndquadrant.com>,
 Alvaro Herrera <alvherre@2ndquadrant.com>,
 Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>,
 Thomas Munro <thomas.munro@enterprisedb.com>,
 Bruce Momjian <bruce@momjian.us>, Mark Dilger <hornschnorter@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: <19f76496-dcf3-ccea-dd82-26fbed57b8f5@2ndquadrant.com>
 <20190313.131951.178195776.horiguchi.kyotaro@lab.ntt.co.jp>
 <CAKJS1f_6qDQj9m2H0jF4bRkZVLpfc7O9E+MxdXrq0wgv0z1NrQ@mail.gmail.com>
 <20190314.205623.235434357.horiguchi.kyotaro@lab.ntt.co.jp>
 <c2406825-6ebe-26e6-91b3-ba94026546a6@2ndquadrant.com>
 <CAEZATCVazGRDjbZpRF6r-Asiv_-U8vcT-VA0oSZribhhmDUQHQ@mail.gmail.com>
 <37ce47c5-e0cf-c6d3-6923-6b8cfd1cc6b1@2ndquadrant.com>
 <CAEZATCWK9j=tfzysBAgpU6KDZE8JyU8G_09r_GDUwhz42iqn6Q@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Message-ID: <2f373a2e-b014-64b2-6e20-5c6276ed2c2e@2ndquadrant.com>
Date: Sun, 17 Mar 2019 14:19:25 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: 
 <CAEZATCWK9j=tfzysBAgpU6KDZE8JyU8G_09r_GDUwhz42iqn6Q@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 3/17/19 1:14 PM, Dean Rasheed wrote:
> On Sat, 16 Mar 2019 at 23:44, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>>>
>>> 16). This regression test fails for me:
>>>
>>> @@ -654,11 +654,11 @@
>>>  -- check change of unrelated column type does not reset the MCV statistics
>>>  ALTER TABLE mcv_lists ALTER COLUMN d TYPE VARCHAR(64);
>>>  SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a =
>>> 1 AND b = ''1''');
>>>   estimated | actual
>>>  -----------+--------
>>> -        50 |     50
>>> +        11 |     50
>>>  (1 row)
>>>
>>> Maybe that's platform-dependent, given what you said about
>>> reltuples/relpages being reset. An easy workaround for this would be
>>> to modify this test (and perhaps the one that follows) to just query
>>> pg_statistic_ext to see if the MCV statistics have been reset.
>>>
>>
>> Ah, sorry for not explaining this bit - the failure is expected, due to
>> the reset of relpages/reltuples I mentioned. We do keep the extended
>> stats, but the relsize estimate changes a bit. It surprised me a bit,
>> and this test made the behavior apparent. The last patchset included a
>> piece that changes that - if we decide not to change this, I think we
>> can simply accept the actual output.
>>
> 
> I don't think changing the way reltuples is reset ought to be within
> the scope of this patch. There might be good reasons for it being the
> way it is. Perhaps open a discussion on a separate thread?
> 

Agreed, will do.

> As far as this test goes, how about just doing this:
> 
> -- check change of unrelated column type does not reset the MCV statistics
> ALTER TABLE mcv_lists ALTER COLUMN d TYPE VARCHAR(64);
> SELECT stxmcv IS NOT NULL AS has_mcv
>   FROM pg_statistic_ext WHERE stxrelid = 'mcv_lists'::regclass;
> 
> -- check change of column type resets the MCV statistics
> ALTER TABLE mcv_lists ALTER COLUMN c TYPE numeric;
> SELECT stxmcv IS NOT NULL AS has_mcv
>   FROM pg_statistic_ext WHERE stxrelid = 'mcv_lists'::regclass;
> 

OK, that's probably the best thing we can do.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services