Date: Thu, 10 Jun 2021 21:55:46 -0700
From: Noah Misch <noah@leadboat.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: PoC/WIP: Extended statistics on expressions
Message-ID: <20210611045546.GA573364@rfd.leadboat.com>
References: 
 <CAEZATCW2WHfsrbDLzwMvD+kOsBemLhvKwoHGhzR7STtac74XTg@mail.gmail.com>
 <a7d17aec-234d-c7f2-bfdc-99c87767a507@enterprisedb.com>
 <1ed534fb-b42b-f008-35ee-4eb930b0088f@enterprisedb.com>
 <CAEZATCV-JgFzb6aCJxApXg302u4vw+ajj702RHaBVkS_4JPVRg@mail.gmail.com>
 <2cb8a7cd-664f-8681-6ce9-790a50e3c78f@enterprisedb.com>
 <CAEZATCXsfFcXW_A1XCd5QK6zT2M_V+P3aPSktjibHHPzoV+Y-A@mail.gmail.com>
 <096d26e1-f053-a039-047d-a050dbb80ac6@enterprisedb.com>
 <078de413-8e4e-343e-fac1-eb61fd389925@enterprisedb.com>
 <20210606053740.GB297923@rfd.leadboat.com>
 <4c98203c-8364-bb76-b502-d7b323ba55ba@enterprisedb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4c98203c-8364-bb76-b502-d7b323ba55ba@enterprisedb.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20210611045546.GA573364%40rfd.leadboat.com>
Precedence: bulk

On Sun, Jun 06, 2021 at 09:13:17PM +0200, Tomas Vondra wrote:
> 
> On 6/6/21 7:37 AM, Noah Misch wrote:
> > On Sat, Mar 27, 2021 at 01:17:14AM +0100, Tomas Vondra wrote:
> >> OK, pushed after a bit more polishing and testing.
> > 
> > This added a "transformed" field to CreateStatsStmt, but it didn't mention
> > that field in src/backend/nodes.  Should those functions handle the field?
> > 
> 
> Yup, that's a mistake - it should do whatever CREATE INDEX is doing. Not
> sure if it can result in error/failure or just inefficiency (due to
> transforming the expressions repeatedly), but it should do whatever
> CREATE INDEX is doing.
> 
> Thanks for noticing! Fixed by d57ecebd12.

Great.  For future reference, this didn't need a catversion bump.  readfuncs.c
changes need a catversion bump, since the catalogs might contain input for
each read function.  Other src/backend/nodes functions don't face that.  Also,
src/backend/nodes generally process fields in the order that they appear in
the struct.  The order you used in d57ecebd12 is nicer, being more like
IndexStmt, so I'm pushing an order change to the struct.