Date: Fri, 1 Mar 2024 11:13:57 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Corey Huinker <corey.huinker@gmail.com>
Cc: Stephen Frost <sfrost@snowman.net>,
	Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
	Peter Smith <smithpb2250@gmail.com>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Statistics Import and Export
Message-ID: <20240301171357.GA3009259@nathanxps13>
References: 
 <CAHut+Pu=XcxBW-mNrhP7ba4kf_o0GN4PQa=MvPa6VV_dckF_Yg@mail.gmail.com>
 <CADkLM=ed0kbWWjvdyecFpZ+gSNQbsVTJwo7AexGw1sPCVNJwkA@mail.gmail.com>
 <6b942362-cd19-4e1c-b6da-f230c3f3d9a1@enterprisedb.com>
 <CADkLM=doQW3TE_LyBwG10NppKZLz0CjAgu10O79uMQc2frYCmA@mail.gmail.com>
 <CADkLM=cfZC6w07Yd_3uK=3xy9pgX+X_1xkEROPvxBHHTMab7-A@mail.gmail.com>
 <CADkLM=c7sMxeOGMmV6MtsjFkNnji6XLfj2bTgy04f4vWWhv6PA@mail.gmail.com>
 <ZeDnq/10RBRM/lB9@tamriel.snowman.net>
 <CADkLM=c8waqcMg+tidQmRudS8cziwPHoAQoLq4zKeEbskzVX2Q@mail.gmail.com>
 <CAOuzzgp4vbs5wOUFyE9Ts+kc7+BU-_5oPNs2dgQ5cHhJBUvmFg@mail.gmail.com>
 <CADkLM=dUGyQMK8Rhz1gVLeW8u2nBWOLvH7-j82MAAN_PmcSAQw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CADkLM=dUGyQMK8Rhz1gVLeW8u2nBWOLvH7-j82MAAN_PmcSAQw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20240301171357.GA3009259%40nathanxps13>
Precedence: bulk

On Thu, Feb 29, 2024 at 10:55:20PM -0500, Corey Huinker wrote:
>> That’s certainly a fair point and my initial reaction (which could
>> certainly be wrong) is that it’s unlikely to be an issue- but also, if you
>> feel you could make it work with an array and passing all the attribute
>> info in with one call, which I suspect would be possible but just a bit
>> more complex to build, then sure, go for it. If it ends up being overly
>> unwieldy then perhaps the  per-attribute call would be better and we could
>> perhaps acquire the lock before the function calls..?  Doing a check to see
>> if we have already locked it would be cheaper than trying to acquire a new
>> lock, I’m fairly sure.
> 
> Well the do_analyze() code was already ok with acquiring the lock once for
> non-inherited stats and again for inherited stats, so the locks were
> already not the end of the world. However, that's at most a 2x of the
> locking required, and this would natts * x, quite a bit more. Having the
> procedures check for a pre-existing lock seems like a good compromise.

I think this is a reasonable starting point.  If the benchmarks show that
the locking is a problem, we can reevaluate, but otherwise IMHO we should
try to keep it as simple/flexible as possible.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com