| 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
| 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. |