From: Tom Lane <tgl@sss.pgh.pa.us>
To: Corey Huinker <corey.huinker@gmail.com>
cc: Magnus Hagander <magnus@hagander.net>, Jeff Davis <pgsql@j-davis.com>,
        Stephen Frost <sfrost@snowman.net>,
        Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
        Matthias van de Meent <boekewurm+postgres@gmail.com>,
        Peter Smith <smithpb2250@gmail.com>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
        Tomas Vondra <tomas.vondra@enterprisedb.com>
Subject: Re: Statistics Import and Export
In-reply-to: 
 <CADkLM=dKC76yUJDWHnL61zEGRR2D6KOf0DM5r23m2M=WFGpZyA@mail.gmail.com>
References: <Zei1xMGH8SJS9yoc@tamriel.snowman.net>
 <54f190b6c757da2a33cce811949e9dd9601fcba7.camel@j-davis.com>
 <1632327b74a734f8c434eb75e824160d2b06a697.camel@j-davis.com>
 <CADkLM=dOUfTtpmE70mx5SmdEs11hic61G-wPa45pB1iC3NRABg@mail.gmail.com>
 <16412b8b7ae443cb7470006beff3011927048860.camel@j-davis.com>
 <CADkLM=cq1n2wbxhV23pOeUKReBPZxZyELDsDJ3VAxkYBSVAe0g@mail.gmail.com>
 <CADkLM=d_WYoFq75eoNHso+9EFDi=Q2BtCmxQYMjrgLL4=v5dxw@mail.gmail.com>
 <1b1bd1ec39ab3de09a17bf7b241021067e5bd63c.camel@j-davis.com>
 <CADkLM=eJXPQD=pWwwm+4Y48haiMmznB8dCECNMrwr59niY8ESw@mail.gmail.com>
 <2bce855db8259c5672a09f863a307ca07b967b68.camel@j-davis.com>
 <CADkLM=dw=fkvPRmOw74UDYnLzXwkDyMSY1k2jsU5=xXBAj7Mpw@mail.gmail.com>
 <5e94c358caedf2a231dd71624c3578db7227a265.camel@j-davis.com>
 <CADkLM=dztWa9shWHyq=+rS9KvbZQ2v54tnOcYT+g8psf0B7w_A@mail.gmail.com>
 <CADkLM=fFwkd7VbO4SPCyX-2f-fyGraka5J+5fcp1QMrCefu68Q@mail.gmail.com>
 <CADkLM=cioQmOFh98qQmqVEkVPjLYszEhZtNtUbmmNAdHOSr3WQ@mail.gmail.com>
 <CADkLM=e=_6dtacmrvd!
 2NJWacOnQ3Zu5iaRZFgePL1=0L5-7P_w@mail.gmail.com>
 <3df03aa65aa6ea3a76582e1a792e422dc5c32ad3.camel@j-davis.com>
 <CAOuzzgpfm9hROr_g8LouvpOeqv4yxk3qXPXU9Sn2nnqzPpP5UA@mail.gmail.com>
 <718fa933c2c58d0ecc61995ab5c7ca1fe78d6369.camel@j-davis.com>
 <CADkLM=f9FOu9Z2GT1cbtRaffDvtXyUOuv2HMVw0=phYe2T2D9w@mail.gmail.com>
 <CABUevEz1gLOkWSh_Vd9LQh-JM4i=Mu7PVT9ffc77TmH0Zh3TzA@mail.gmail.com>
 <CADkLM=dTY5bkMB4WO0aTYEOP=re0_QjWfYyPd-A5-BvEQdn9cA@mail.gmail.com>
 <790773.1711910899@sss.pgh.pa.us>
 <CADkLM=dracL=rSf+2v+1P4NN_4jq--kyv6Ywh77t4MiiRX98og@mail.gmail.com>
 <864345.1711932452@sss.pgh.pa.us>
 <CADkLM=dKC76yUJDWHnL61zEGRR2D6KOf0DM5r23m2M=WFGpZyA@mail.gmail.com>
Comments: In-reply-to Corey Huinker <corey.huinker@gmail.com>
	message dated "Sun, 31 Mar 2024 21:32:17 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1043792.1711984209.1@sss.pgh.pa.us>
Date: Mon, 01 Apr 2024 11:10:09 -0400
Message-ID: <1043793.1711984209@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1043793.1711984209%40sss.pgh.pa.us>
Precedence: bulk

Corey Huinker <corey.huinker@gmail.com> writes:
>> IIRC, "variadic any" requires having at least one variadic parameter.
>> But that seems fine --- what would be the point, or even the
>> semantics, of calling pg_set_attribute_stats with no data fields?

> If my pg_dump run emitted a bunch of stats that could never be imported,
> I'd want to know. With silent failures, I don't.

What do you think would be silent about that?  If there's a complaint
to be made, it's that it'd be a hard failure ("no such function").

To be clear, I'm ok with emitting ERROR for something that pg_dump
clearly did wrong, which in this case would be emitting a
set_statistics call for an attribute it had exactly no stats values
for.  What I think needs to be WARN is conditions that the originating
pg_dump couldn't have foreseen, for example cross-version differences.
If we do try to check things like sort order, that complaint obviously
has to be WARN, since it's checking something potentially different
from what was correct at the source server.

>> Perhaps we could
>> invent a new backend function that extracts the actual element type
>> of a non-null anyarray argument.

> A backend function that we can't guarantee exists on the source system. :(

[ shrug... ] If this doesn't work for source servers below v17, that
would be a little sad, but it wouldn't be the end of the world.
I see your point that that is an argument for finding another way,
though.

>> Another way we could get to no-coercions is to stick with your
>> signature but declare the relevant parameters as anyarray instead of
>> text.

> I'm a bit confused here. AFAIK we can't construct an anyarray in SQL:

> # select '{1,2,3}'::anyarray;
> ERROR:  cannot accept a value of type anyarray

That's not what I suggested at all.  The function parameters would
be declared anyarray, but the values passed to them would be coerced
to the correct concrete array types.  So as far as the coercion rules
are concerned this'd be equivalent to the variadic-any approach.

> That's pretty persuasive. It also means that we need to trap for error in
> the array_in() calls, as that function does not yet have a _safe() mode.

Well, the approach I'm advocating for would have the array input and
coercion done by the calling query before control ever reaches
pg_set_attribute_stats, so that any incorrect-for-the-data-type values
would result in hard errors.  I think that's okay for the same reason
you probably figured you didn't have to trap array_in: it's the fault
of the originating pg_dump if it offers a value that doesn't coerce to
the datatype it claims the value is of.  My formulation is a bit safer
though in that it's the originating pg_dump, not the receiving server,
that is in charge of saying which type that is.  (If that type doesn't
agree with what the receiving server thinks it should be, that's a
condition that pg_set_attribute_stats itself will detect, and then it
can WARN and move on to the next thing.)

			regards, tom lane