Re: Further cleanup related to statistics import support in postgres_fdw

On Thu, Aug 20, 2026 at 8:55 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> On Thu, Aug 20, 2026 at 3:44 AM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > On Wed, Aug 19, 2026 at 4:44 AM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> > > Here is a patch for $SUBJECT. Changes are:

> > > * Rename a variable to match other places.
> > > * Fix typos in comments.
> >
> > These seem fine to me and can go separately.
>
> Cool! I will push/backpatch the typo-fix part separately. The
> renaming part is a code change, so I'd like to merge it into other
> code changes.

I pushed/backpatched the typo fix.

> > > * Add/Tweak some comments/docs for clarity.
> >
> > A few comments:

> > 2/
> > - /* Fetch relation stats. */
> > + /* Fetch relation statistics. */
> > remstats->rel = relstats = fetch_relstats(conn, relation);
> >
> > /*
> > - * Attempt to fetch remote attribute stats.
> > + * Attempt to fetch remote attribute statistics.
> > */
> > static PGresult *
> > fetch_attstats(PGconn *conn, int server_version_num,
> >
> > These are fine as-is IMO, the use of "stats" is widespread in the code comments.
>
> Ok, I will refrain from changing.

Removed.

> > 3/
> > +/*
> > + * Determine whether the column is analyzable.
> > + *
> > + * If the column is analyzable, its attstattarget value is returned into the
> > + * output parameter p_attstattarget if it isn't NULL.
> > + */
> > bool
> > attribute_is_analyzable(Relation onerel, int attnum, Form_pg_attribute attr,
> >
> > How about just saying "If the column is analyzable, get its
> > attstattarget value if asked" without describing what the code does in
> > detail.
>
> Seems like a good idea. Will change.

Modified. I tweaked the wording a little bit, though.

Attached is an updated version of the patch.

Best regards,
Etsuro Fujita