Subject: Re: PoC/WIP: Extended statistics on expressions
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: <f0f7e00c-7ccb-9412-0147-a06a60777f06@enterprisedb.com>
 <f4dac079-6bc4-ccc0-0fd8-3e2e2da28d92@enterprisedb.com>
 <CAEZATCXz2WO9ZctRL68_HrsPbpre0GRmip83TgznoeLoWm4=Dg@mail.gmail.com>
 <CAEZATCXcX0gm2LMb+xObroS-C+a5FXEqgR5RgQ9NWKjPC=9sNA@mail.gmail.com>
 <ce50ee26-7135-c435-cae8-073ad59eba60@enterprisedb.com>
 <CAEZATCWgZPJGbj2ndXsoD6_MuJ=H1Y-gdTMdkE=n1VsA6WX+RA@mail.gmail.com>
 <9f6654b1-ca31-f979-0fe8-137683818273@enterprisedb.com>
 <20210324081402.GB28335@telsasoft.com>
 <b8359875-b91f-ff48-4c6c-9b3c1c35d880@enterprisedb.com>
 <20210324091708.GC28335@telsasoft.com>
 <1ff3fc47-8f6e-7839-c3ff-6b694956f92e@enterprisedb.com>
 <CAEZATCWYCY=2yEyxa6BwimCPQ_RZHy5GOb6U6i6dGHGQdjQESQ@mail.gmail.com>
 <e4556e47-940d-58a6-73fd-16c5815dd2cd@enterprisedb.com>
 <CAEZATCW2WHfsrbDLzwMvD+kOsBemLhvKwoHGhzR7STtac74XTg@mail.gmail.com>
 <a7d17aec-234d-c7f2-bfdc-99c87767a507@enterprisedb.com>
 <1ed534fb-b42b-f008-35ee-4eb930b0088f@enterprisedb.com>
Message-ID: <fc7ec493-c7cc-1db0-5e1d-9251c5e51387@enterprisedb.com>
Date: Thu, 25 Mar 2021 05:07:17 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <1ed534fb-b42b-f008-35ee-4eb930b0088f@enterprisedb.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/fc7ec493-c7cc-1db0-5e1d-9251c5e51387%40enterprisedb.com>
Precedence: bulk

On 3/25/21 1:05 AM, Tomas Vondra wrote:
> ...
>
> 0002 is an attempt to fix an issue I noticed today - we need to handle
> type changes. Until now we did not have problems with that, because we
> only had attnums - so we just reset the statistics (with the exception
> of functional dependencies, on the assumption that those remain valid).
> 
> With expressions it's a bit more complicated, though.
> 
> 1) we need to transform the expressions so that the Vars contain the
> right type info etc. Otherwise an analyze with the old pg_node_tree crashes
> 
> 2) we need to reset the pg_statistic[] data too, which however makes
> keeping the functional dependencies a bit less useful, because those
> rely on the expression stats :-(
> 
> So I'm wondering what to do about this. I looked into how ALTER TABLE
> handles indexes, and 0003 is a PoC to do the same thing for statistics.
> Of couse, this is a bit unfortunate because it recreates the statistics
> (so we don't keep anything, not even functional dependencies).
> 
> I think we have two options:
> 
> a) Make UpdateStatisticsForTypeChange smarter to also transform and
> update the expression string, and reset pg_statistics[] data.
> 
> b) Just recreate the statistics, just like we do for indexes. Currently
> this does not force analyze, so it just resets all the stats. Maybe it
> should do analyze, though.
> 
> Any opinions? I need to think about this a bit more, but maybe (b) with
> the analyze is the right thing to do. Keeping just some of the stats
> always seemed a bit weird. (This is why the 0002 patch breaks one of the
> regression tests.)
> 

After thinking about this a bit more I think (b) is the right choice,
and the analyze is not necessary. The reason is fairly simple - we drop
the per-column statistics, because ATExecAlterColumnType does

    RemoveStatistics(RelationGetRelid(rel), attnum);

so the user has to run analyze anyway, to get any reasonable estimates
(we keep the functional dependencies, but those still rely on per-column
statistics quite a bit). And we'll have to do the same thing with
per-expression stats too. It was a nice idea to keep at least the stats
that are not outright broken, but unfortunately it's not a very useful
optimization. It increases the instability of the system, because now we
have estimates with all statistics, no statistics, and something in
between after the partial reset. Not nice.

So my plan is to get rid of UpdateStatisticsForTypeChange, and just do
mostly what we do for indexes. It's not perfect (as demonstrated in last
message), but that'd apply even to option (a).

Any better ideas?


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company