Re: Import Statistics in postgres_fdw before resorting to sampling.

From: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
To: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)postgresql(dot)org, jkatz(at)postgresql(dot)org, nathandbossart(at)gmail(dot)com
Subject: Re: Import Statistics in postgres_fdw before resorting to sampling.
Date: 2026-03-30 09:42:09
Message-ID: CAPmGK17mMObnAeQqdkGUEGLvzTAHbjUOAYYixFNiY5fyUTrxzg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Mar 15, 2026 at 4:51 AM Corey Huinker <corey(dot)huinker(at)gmail(dot)com> wrote:
>> I too was thinking of network timeouts (and in such a case I thought
>> that even if PQsendQuery was run successfully, the following
>> PQgetResult would have to block for a long time). And yes, that
>> situation can occur for normal queries, but in such a query, the extra
>> functions call is done only when aborting the remote transaction, in a
>> way libpq functions don't have to wait for a network timeout.

> I've copied the ERROR-behavior that was already done in postgresGetAnalyzeInfoForForeignTable(), which I assume will be to your liking.
>
> Structural error checks (how many columns does the result set have, etc) are now elog errors, bad PQresultStatus-es are now errors as well.

The patch looks good to me other than this bit in fetch_relstats/fetch_attstats:

+ res = pgfdw_get_result(conn);
+
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(NULL, conn, sql);

You forgot to pass the res to pgfdw_report_error for proper logging.
I fixed this in the patch I posted just before.

>> I forgot to mention this, but when we have reltuples = 0 for v14 or
>> later, the patch tries to import both relstats and attstats, but in
>> that case I think it would be OK to do so only for the former for the
>> consistency with the normal processing. What do you think about that?

> I've update the logic that we don't try to fetch attrstats if the reltuples is 0 or -1, and though the comments still mention the difference in server version, the code behaves the same in new versions and old. My thinking is that either value means "you are not going to find attstats" and our reaction is the same either way.

OK, but I modified this further in the patch. Please check it.

Thanks again!

Best regards,
Etsuro Fujita

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2026-03-30 09:44:33 Re: [PATCH] Add CANONICAL option to xmlserialize
Previous Message Etsuro Fujita 2026-03-30 09:32:38 Re: Import Statistics in postgres_fdw before resorting to sampling.