Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1weanx-004tCR-36 for pgsql-hackers@arkaria.postgresql.org; Tue, 30 Jun 2026 15:55:22 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1weanv-008zNh-1l for pgsql-hackers@arkaria.postgresql.org; Tue, 30 Jun 2026 15:55:19 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1weanv-008zNY-03 for pgsql-hackers@lists.postgresql.org; Tue, 30 Jun 2026 15:55:19 +0000 Received: from mail-dl1-x122c.google.com ([2607:f8b0:4864:20::122c]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1weans-00000000wH7-3Dhg for pgsql-hackers@postgresql.org; Tue, 30 Jun 2026 15:55:18 +0000 Received: by mail-dl1-x122c.google.com with SMTP id a92af1059eb24-13b305a5301so914962c88.1 for ; Tue, 30 Jun 2026 08:55:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1782834916; cv=none; d=google.com; s=arc-20260327; b=dfYFY5OmMCLhA7PlywaykNBlsZSXxFEv2I+EUGTIEIyODfsX4PvhumWeoGKQbDXkca Sku/0zl03gunms9W0/YCqtTqr+adDjxjVb4YF3ARDpXtQqDmsSbo+lmXhB/SsLho1kP/ fVBqpFLT4lupKCEoAnXmrZKI+iLCjLgXnYQOeI0CqU9kxhf6rKBZCclsMNNQZXC/t4rh WoyUNvm+XPPnshSPSFLgNkuJ1zh1xm2tDS+AvkjllvtVg44iM3Duzbr8hplHhDilGlBd 8DG+B4ySlYH9MoTKB16TQxf4jwy2FiXLxGaMcYRedoSvmXL5Cr2+Dygw9FkmtTMqFQZG ju7A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=sZ/Jusv8eaQuwzcwjy0ZET3jhvB+UijvgB7kZvgsQI4=; fh=u/l0HIZDgSN867KdHHcabM6YoK6nEOXqrgMYuW20ZFc=; b=T7+CDAQyyGWUDF1DMyGpqeLVeiO4hXt/PZIzacb+2tq87JkiQ7spbqjFWb4gE/v80I A/E6dYxnMir/5hyjz9FQ6H7ldwbMT52L6f5S+YwHiCv8AAH7gYIHOP6miKsQTyhTmj3M w0/UMOQDAvkarUZu8tLd2k/j/0W6Mzd2WUXoyNP4u1e57UJOizpWkRL9Nhxm7rT7cO6F nw1WqPbGW8QO2lj18zym6jIo6+3Qv+ZJPA5/YtwQZsdpMBnxv597N6C0kvLo1Jhd42G8 3tUvzWHMIX/d74XToeQhAZFh+loC0nlq1f/yWfjsNnto064vcyARHL9g3Lh1U9WnL/y6 0gVA==; darn=postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1782834916; x=1783439716; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=sZ/Jusv8eaQuwzcwjy0ZET3jhvB+UijvgB7kZvgsQI4=; b=MwS6RmT/EdKIAn19a44oa+JfQ3KhTQrRW+BeZ7IChLP93oL1hOR9KzpZ/ZnfRwTw6e pLt2G2R9EAyiGmdyC0G7GwDBZC4rTr1yMnDXkgJNpau52r9jb788Ddsv+v75Nvi2gRgH LIU6lA4QvBBi3WqCb85dzxqBueO4BzZxMghRPFNfsjnCpVUqRYygZFNpxgygK+UfKPwi OCiYpqnfm+sVn1A/CYMpVF1snfDZv/G0s3X1qHFhP11u5/ESGeJZlHi7XdFJHiWGr4JM e4X1GlwvErMIz4nx5lJX6zwZ2Xksu8CTFb3jyfNzed9pdHaaK6oXy/ERbabDLnlBWCXi Wv+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782834916; x=1783439716; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=sZ/Jusv8eaQuwzcwjy0ZET3jhvB+UijvgB7kZvgsQI4=; b=Y2rFtFMfWsntp32Qzkc8mOquU8/v/+kaNgI0RwuzI9WyU6K08747R2F2VnRuf2UL1i uqU8z5/ti/Ah8+P1QXzEDzIJGFSxQTZ/TsAxTxCmqYe6vI8pRPiGdQGFnaLhX3JuvPPk tCBhI00EzIqA5DGfKVJnkFsdp9gxdTd2HeeIebEJ/fxc6DrfxmPr4iUpolNEkw2t2gg2 mTMZHtaULREaupFXQhW9MnZ9OOg+y/oxb3htUh7vtMzdmDR8pfP0A+BIJiDOl/iz7p0l K5wIchAZwjQ1ojzmn66PBGKrgIP2Ko6ecdoYFH0QCVGKFDCTz1C3kEYQ+IBPip9V1sGv +SMQ== X-Forwarded-Encrypted: i=1; AFNElJ8Pn/yTKPyYR+SuyQgxjCyhLfITFCzbWMuzlRI7a7jv7LUtUNK9St9Hh5uqWgvK88Rru245l9jw80GbX1AH@postgresql.org X-Gm-Message-State: AOJu0Ywdi6eHhicc0Cb9ZqNmyo4jXQaqL1HGyr0GHPqiZTutPvOGIW9C /h0x1yDVIMa1Y6pSpgJNKs+0jLn/a9TStjc4EKUvAOxuEJa7B1U5rH/LChcZKiIXChmNAbWRmtt 8CmKb4xB22z7n+jSYrFeEmv3JUjgPpk0= X-Gm-Gg: AfdE7ckbmjwye8naXIlBr33UKOmFfWgVtqdxaKnUa4YthxY9XS/vJtP/ElREgSUYyue s273kD5oi4twvrkIOUj73hkeJbT3rTnybvzPLaCTY4pG239GaLsJDbrtBC512cN3hWQdIjihCnP I8FiroJnBiJfivaHd4P5nFuLo7ZF14ej5YgmLe0/4wuaE4wFA7qRo7iZdzNWNfsrcUrbx0Ijcxn IsG0v0m0IrgDH6UGSmFVLzI57j73waD6i5lvTKfZBee9mFMYHaIWc1yPmFgDUcutXIlxtbQcr9S zFuNfFcNcmTmfGtYDjeJ0oQY1LU0qEzUoFhmdkznfK2kVPU4WmxeIQ3QMtJPsoEYZF6LV/0X373 N/8gawDJkJs9aCWCXcZqee5hpjJhETJnlKJUqYB2Q+Uu/cOrp X-Received: by 2002:a05:7022:ec84:b0:138:33cc:d88b with SMTP id a92af1059eb24-13b31437c22mr654008c88.20.1782834915711; Tue, 30 Jun 2026 08:55:15 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Corey Huinker Date: Tue, 30 Jun 2026 10:55:03 -0500 X-Gm-Features: AVVi8CdNlbcJIfPRz2rSOfVIR2wI63tok_bCRwG4CMylNyN4X29Y12zJ8kxHB4I Message-ID: Subject: Re: use of SPI by postgresImportForeignStatistics To: Etsuro Fujita Cc: Robert Haas , "pgsql-hackers@postgresql.org" Content-Type: multipart/alternative; boundary="000000000000d821a106557a99ad" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000d821a106557a99ad Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Jun 30, 2026 at 7:30=E2=80=AFAM Etsuro Fujita wrote: > On Tue, Jun 30, 2026 at 10:10=E2=80=AFAM Corey Huinker > wrote: > > On Mon, Jun 29, 2026 at 2:17=E2=80=AFPM Robert Haas > wrote: > >> On Mon, Jun 29, 2026 at 7:50=E2=80=AFAM Etsuro Fujita > wrote: > >> > I like this refactoring, but while it's rather mechanical, it's pret= ty > >> > large, so I think it's too late to do the refactoring at this time > >> > just before the beta 2 release. So I'd vote for going with your > >> > v1-0001 and v1-0002 and doing the refactoring in v20. As mentioned = by > >> > Robert, I don't think it's good to call LOCAL_FCINFO() in > >> > import_relation_statistics() and import_attribute_statistics() to ca= ll > >> > the guts of those functions either, but that is *consistent* with th= e > >> > existing way pg_restore_relation_stats() and > >> > pg_restore_attribute_stats() do that, so that is actually not that > >> > bad. Also, as you mentioned above, it's inefficient for the new API > >> > functions to lock an already-locked relation, and validate an > >> > already-validated attname/attnum, but I think it would be negligible= . > >> > >> Fujita-san, is it then your plan to get those two patches committed? > > Yes, it is. > > > Are we ok with changing the functions that postgres_fdw makes from v19 > to v20? Because what I put in the v2 patch does not match the v1 patch. I= f > we are NOT ok with that, then we'd need to: > > > > 1. rename the two relation_stats_argnum -> relation_args_argnum and > attribute_stats_argnum -> attribute_args_argnum out of the way of the one= s > created in relation_stats.h and attribute_stats.h. The actual enumeration > values can stay the same, as there are no conflicts there, just ambiguity > about which set of values they belong to. > > 2. rename the existing static functions relation_statistics_update -> > update_relstats and attribute_statistics_update -> update_relstats to mak= e > way for the public functions of the same names. > > 3. Create new relation_statistics_update and > attribute_statistics_update, with the isnull/values, and have those > fcinfo-invoke the respective pg_restore functions just like v1 does. > > > > That would at least make the user API consistent in v19 vs v20. > > > > I'm on vacation this week, but time is short for beta2, so I'll make an > exception to get the above into beta2. Let me know if you want me to writ= e > that up. > > Thanks! > > Here is an updated patch, which merges your v1-0001 and v1-0002. One > notable change I made to your version is the signatures of > import_relation_statistics() and import_attribute_statistics(). As > mentioned upthread, the proposed signatures of these functions would > be too tailored for postgres_fdw, and might not be useful for other > FDWs, so I changed them to have NullableDatum arguments for stats-data > inputs like this: > > bool > import_relation_statistics(Relation rel, > NullableDatum *relpages, > NullableDatum *reltuples, > NullableDatum *relallvisible, > NullableDatum *relallfrozen) > > bool > import_attribute_statistics(Relation rel, AttrNumber attnum, bool > inherited, > NullableDatum *null_frac, > NullableDatum *avg_width, > NullableDatum *n_distinct, > NullableDatum *most_common_vals, > NullableDatum *most_common_freqs, > NullableDatum *histogram_bounds, > NullableDatum *correlation, > NullableDatum *most_common_elems, > NullableDatum *most_common_elem_freqs, > NullableDatum *elem_count_histogram, > NullableDatum *range_length_histogram, > NullableDatum *range_empty_frac, > NullableDatum *range_bounds_histogram) > > As postgresImportForeignStatistics() is provided for FDWs that want to > calculate statistics remotely, so I'm assuming that they have enough > knowledge about the stats-data structures. > I think we could instead > use the values/isnull arrays as proposed in your v2, but the problem > about doing so is error messages in functions in stat_utils.c like > this: > > ereport(WARNING, > (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > errmsg("argument \"%s\" must not be a multidimensional > array", > arginfo[argnum].argname))); > > Note that the message uses the word "argument", so I think it's better > for the new functions as well to have individual arguments for > stats-data inputs. That's a good example of how our error messages get a bit clumsy with so many ways of calling things, so I'm fine with your solution. It also eliminates the possibility that a caller fails to notice a newly added stat parameter and instead calls the input function with the older short array length. My initial concern was that this would bloat up postgres_fdw with type conversion code, but the patch shows that it isn't that bad. One thing we do lose in this is the assurance that the Datums passed are of the correct type. Presently that is checked via stats_fill_fcinfo_from_arg_pairs, which allows the *_statistics_update() functions to trust the type correctness of the Datums that they're passed, but I can be convinced that it isn't a big problem. > Also, I removed the attname argument from > import_attribute_statistics(), to 1) make the signature (and the > internal processing) simple and 2) match it to > delete_attribute_statistics(). > I think that's reasonable. Internal callers have already resolved which attribute they want to modify. The attname parameter was there so that pg_restore_* calls could survive dump/restores where there were dropped columns so the attnum has changed. In relation to this change, I moved helper functions set_XXX_arg() > that you added to relation_stats.c and attribute_stats.c to > postgres_fdw.c. The helper functions had soft error handling, but in > the postgres_fdw use, there is no need for that, so I removed that > handling as well. > My inclination would be to move the set_*_arg functions could be moved to some common utility file in v20, as other FDWs will find themselves with the same need. As for removing the error-handling, it might still be handy because it would allow us to give a more context-aware error message, rather than the very narrow "this_string is an invalid this_type", for string that the user most certainly never saw. Having said that, it's something we could easily change back to error-safe if we wanted to. What do you think? If there is no problem with this change, we > wouldn't need to worry about the stability of postgres_fdw (and other > FDWs) in v20. > If you're fine with the names import_relation_statistics and import_attribute_statistics, then yes. Those names made sense to me in the narrow case of being called from an FDW handler and handing the function a bunch of strings, but in a more general case with properly typed datums, the name feels less appropriate, but it's just a name and I wouldn't let it get in the way of the overall patch. --000000000000d821a106557a99ad Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Tue, Jun 30, 2026 at = 7:30=E2=80=AFAM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
On Tue, = Jun 30, 2026 at 10:10=E2=80=AFAM Corey Huinker <corey.huinker@gmail.com> wrote:=
> On Mon, Jun 29, 2026 at 2:17=E2=80=AFPM Robert Haas <robertmhaas@gmail.com> = wrote:
>> On Mon, Jun 29, 2026 at 7:50=E2=80=AFAM Etsuro Fujita <etsuro.fujita@gmail.c= om> wrote:
>> > I like this refactoring, but while it's rather mechanical= , it's pretty
>> > large, so I think it's too late to do the refactoring at = this time
>> > just before the beta 2 release.=C2=A0 So I'd vote for goi= ng with your
>> > v1-0001 and v1-0002 and doing the refactoring in v20.=C2=A0 A= s mentioned by
>> > Robert, I don't think it's good to call LOCAL_FCINFO(= ) in
>> > import_relation_statistics() and import_attribute_statistics(= ) to call
>> > the guts of those functions either, but that is *consistent* = with the
>> > existing way pg_restore_relation_stats() and
>> > pg_restore_attribute_stats() do that, so that is actually not= that
>> > bad.=C2=A0 Also, as you mentioned above, it's inefficient= for the new API
>> > functions to lock an already-locked relation, and validate an=
>> > already-validated attname/attnum, but I think it would be neg= ligible.
>>
>> Fujita-san, is it then your plan to get those two patches committe= d?

Yes, it is.

> Are we ok with changing the functions that postgres_fdw makes from v19= to v20? Because what I put in the v2 patch does not match the v1 patch. If= we are NOT ok with that, then we'd need to:
>
> 1. rename the two relation_stats_argnum -> relation_args_argnum and= attribute_stats_argnum -> attribute_args_argnum out of the way of the o= nes created in relation_stats.h and attribute_stats.h. The actual enumerati= on values can stay the same, as there are no conflicts there, just ambiguit= y about which set of values they belong to.
> 2. rename the existing static functions relation_statistics_update -&g= t; update_relstats and attribute_statistics_update -> update_relstats to= make way for the public functions of the same names.
> 3. Create new relation_statistics_update and attribute_statistics_upda= te, with the isnull/values, and have those fcinfo-invoke the respective pg_= restore functions just like v1 does.
>
> That would at least make the user API consistent in v19 vs v20.
>
> I'm on vacation this week, but time is short for beta2, so I'l= l make an exception to get the above into beta2. Let me know if you want me= to write that up.

Thanks!

Here is an updated patch, which merges your v1-0001 and v1-0002.=C2=A0 One<= br> notable change I made to your version is the signatures of
import_relation_statistics() and import_attribute_statistics().=C2=A0 As mentioned upthread, the proposed signatures of these functions would
be too tailored for postgres_fdw, and might not be useful for other
FDWs, so I changed them to have NullableDatum arguments for stats-data
inputs like this:

bool
import_relation_statistics(Relation rel,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0NullableDatum *relpages,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0NullableDatum *reltuples,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0NullableDatum *relallvisible,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0NullableDatum *relallfrozen)

bool
import_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited= ,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *null_frac,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *avg_width,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *n_distinct,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *most_common_vals,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *most_common_freqs,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *histogram_bounds,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *correlation,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *most_common_elems,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *most_common_elem_freqs,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *elem_count_histogram,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *range_length_histogram,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *range_empty_frac,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 NullableDatum *range_bounds_histogram)

As postgresImportForeignStatistics() is provided for FDWs that want to
calculate statistics remotely, so I'm assuming that they have enough knowledge about the stats-data structures.

= =C2=A0
=C2=A0 I thin= k we could instead
use the values/isnull arrays as proposed in your v2, but the problem
about doing so is error messages in functions in stat_utils.c like
this:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 ereport(WARNING,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (errcode(ERRCODE_IN= VALID_PARAMETER_VALUE),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0errmsg("= argument \"%s\" must not be a multidimensional array",
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 arginfo[argnum].argname)));

Note that the message uses the word "argument", so I think it'= ;s better
for the new functions as well to have individual arguments for
stats-data inputs.

That's a good exampl= e of how our error messages get a bit clumsy with so many ways of calling t= hings, so I'm fine with your solution. It also eliminates the possibili= ty that a caller fails to notice a newly added stat parameter and instead c= alls the input function with the older short array length.

My initial concern was that this would bloat up postgres_fdw with = type conversion code, but the patch shows that it isn't that bad.
=

One thing we do lose in this is the assurance that the = Datums passed are of the correct type. Presently that is checked via stats_= fill_fcinfo_from_arg_pairs, which allows the *_statistics_update() function= s to trust the type correctness of the Datums that they're passed, but = I can be convinced that it isn't a big problem.
=C2=A0
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex"> Also, I removed the attname argument from
import_attribute_statistics(), to 1) make the signature (and the
internal processing) simple and 2) match it to
delete_attribute_statistics().

I think = that's reasonable. Internal callers have already resolved which attribu= te they want to modify. The attname parameter was there so that pg_restore_= * calls could survive dump/restores where there were dropped columns so the= attnum has changed.

In relation to this change, I moved helper functions set_XXX_arg()
that you added to relation_stats.c and attribute_stats.c to
postgres_fdw.c.=C2=A0 The helper functions had soft error handling, but in<= br> the postgres_fdw use, there is no need for that, so I removed that
handling as well.

My inclination w= ould be to move the set_*_arg functions could be=20 moved to some common utility file in v20, as other FDWs will find=20 themselves with the same need.

As for removing the= error-handling, it might still be handy because it would allow us to give = a more context-aware error message, rather than the very narrow "this_= string is an invalid this_type", for string that the user most certain= ly never saw. Having said that, it's something we could easily change b= ack to error-safe if we wanted to.

What do you think?=C2=A0 If there is no problem with this change, we
wouldn't need to worry about the stability of postgres_fdw (and other FDWs) in v20.

If you're fine with t= he names import_relation_statistics and import_attribute_statistics, then y= es. Those names made sense to me in the narrow case of being called from an= FDW handler and handing the function a bunch of strings, but in a more gen= eral case with properly typed datums, the name feels less appropriate, but = it's just a name and I wouldn't let it get in the way of the overal= l patch.
--000000000000d821a106557a99ad--