pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpostgres_fdw: Fix bug in checking of return value of PQsendQuery().
5+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2022-07-22 44ccdce514 63a8c689b0 169d50ba34 postgres_fdw: Fix bug in checking of return value of PQsendQuery().
* postgres_fdw: Fix bug in checking of return value of PQsendQuery().
@ 2022-07-21 14:22 Fujii Masao <masao.fujii@oss.nttdata.com>
2022-07-21 14:41 ` Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Japin Li <japinli@hotmail.com>
2022-07-21 15:22 ` Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 5+ messages in thread
From: Fujii Masao @ 2022-07-21 14:22 UTC (permalink / raw)
To: pgsql-hackers
Hi,
I found that fetch_more_data_begin() in postgres_fdw reports an error when PQsendQuery() returns the value less than 0 as follows though PQsendQuery() can return only 1 or 0. I think this is a bug. Attached is the patch that fixes this bug. This needs to be back-ported to v14 where async execution was supported in postgres_fdw.
if (PQsendQuery(fsstate->conn, sql) < 0)
pgfdw_report_error(ERROR, NULL, fsstate->conn, false, fsstate->query);
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
0001-postgres_fdw-Fix-bug-in-checking-of-return-value-of-.patch (text/plain, 1.6K) download
From 538f33a17f3623cec54768a7325d64f8e97abe06 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Thu, 21 Jul 2022 22:52:50 +0900
Subject: [PATCH] postgres_fdw: Fix bug in checking of return value of
PQsendQuery().
When postgres_fdw begins an asynchronous data fetch, it submits FETCH query
by using PQsendQuery(). If PQsendQuery() fails and returns 0, postgres_fdw
should report an error. But, previously, postgres_fdw reported an error
only when the return value is less than 0, though PQsendQuery() never return
the values other than 0 and 1. Therefore postgres_fdw could not handle
the failure to send FETCH query in an asynchronous data fetch.
This commit fixes postgres_fdw so that it reports an error
when PQsendQuery() returns 0.
Back-patch to v14 where asynchronous execution was supported in postgres_fdw.
Author: Fujii Masao
Reviewed-by:
Discussion: https://postgr.es/m/
---
contrib/postgres_fdw/postgres_fdw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index f3b93954ee..048db542d3 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -7070,7 +7070,7 @@ fetch_more_data_begin(AsyncRequest *areq)
snprintf(sql, sizeof(sql), "FETCH %d FROM c%u",
fsstate->fetch_size, fsstate->cursor_number);
- if (PQsendQuery(fsstate->conn, sql) < 0)
+ if (!PQsendQuery(fsstate->conn, sql))
pgfdw_report_error(ERROR, NULL, fsstate->conn, false, fsstate->query);
/* Remember that the request is in process */
--
2.36.0
* Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().
2022-07-21 14:22 postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2022-07-21 14:41 ` Japin Li <japinli@hotmail.com>
2022-07-22 03:07 ` Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Fujii Masao <masao.fujii@oss.nttdata.com>
1 sibling, 1 reply; 5+ messages in thread
From: Japin Li @ 2022-07-21 14:41 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: pgsql-hackers; pgsql-hackers@lists.postgresql.org
On Thu, 21 Jul 2022 at 22:22, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> Hi,
>
> I found that fetch_more_data_begin() in postgres_fdw reports an error when PQsendQuery() returns the value less than 0 as follows though PQsendQuery() can return only 1 or 0. I think this is a bug. Attached is the patch that fixes this bug. This needs to be back-ported to v14 where async execution was supported in postgres_fdw.
>
> if (PQsendQuery(fsstate->conn, sql) < 0)
> pgfdw_report_error(ERROR, NULL, fsstate->conn, false, fsstate->query);
>
> Regards,
+1. However, I think check whether the result equals 0 or 1 might be better.
Anyway, the patch works correctly.
$ grep 'PQsendQuery(' -rn . --include '*.c'
./contrib/postgres_fdw/postgres_fdw.c:7073: if (PQsendQuery(fsstate->conn, sql) < 0)
./contrib/postgres_fdw/connection.c:647: if (!PQsendQuery(conn, sql))
./contrib/postgres_fdw/connection.c:782: if (!PQsendQuery(conn, query))
./contrib/postgres_fdw/connection.c:1347: if (!PQsendQuery(conn, query))
./contrib/postgres_fdw/connection.c:1575: if (PQsendQuery(entry->conn, "DEALLOCATE ALL"))
./contrib/dblink/dblink.c:720: retval = PQsendQuery(conn, sql);
./contrib/dblink/dblink.c:1146: if (!PQsendQuery(conn, sql))
./src/test/isolation/isolationtester.c:669: if (!PQsendQuery(conn, step->sql))
./src/test/modules/libpq_pipeline/libpq_pipeline.c:500: if (PQsendQuery(conn, "SELECT 1; SELECT 2") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:532: if (PQsendQuery(conn, "SELECT 1.0/g FROM generate_series(3, -1, -1) g") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:1000: if (PQsendQuery(conn, "SELECT 1") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:1046: if (PQsendQuery(conn, "SELECT 1") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:1084: if (PQsendQuery(conn, "SELECT 1") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:1094: if (PQsendQuery(conn, "SELECT 2") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:1118: if (PQsendQuery(conn, "SELECT 1") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:1132: if (PQsendQuery(conn, "SELECT 2") != 1)
./src/test/modules/libpq_pipeline/libpq_pipeline.c:1159: if (PQsendQuery(conn, "SELECT pg_catalog.pg_advisory_unlock(1,1)") != 1)
./src/bin/pg_basebackup/pg_basebackup.c:1921: if (PQsendQuery(conn, basebkp) == 0)
./src/bin/pg_amcheck/pg_amcheck.c:891: if (PQsendQuery(slot->connection, sql) == 0)
./src/bin/psql/common.c:1451: success = PQsendQuery(pset.db, query);
./src/bin/scripts/reindexdb.c:551: status = PQsendQuery(conn, sql.data) == 1;
./src/bin/scripts/vacuumdb.c:947: status = PQsendQuery(conn, sql) == 1;
./src/bin/pgbench/pgbench.c:3089: r = PQsendQuery(st->con, sql);
./src/bin/pgbench/pgbench.c:4012: if (!PQsendQuery(st->con, "ROLLBACK"))
./src/backend/replication/libpqwalreceiver/libpqwalreceiver.c:663: if (!PQsendQuery(streamConn, query))
./src/interfaces/libpq/fe-exec.c:1421:PQsendQuery(PGconn *conn, const char *query)
./src/interfaces/libpq/fe-exec.c:2319: if (!PQsendQuery(conn, query))
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().
2022-07-21 14:22 postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Fujii Masao <masao.fujii@oss.nttdata.com>
2022-07-21 14:41 ` Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Japin Li <japinli@hotmail.com>
@ 2022-07-22 03:07 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2022-07-22 08:03 ` Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Fujii Masao @ 2022-07-22 03:07 UTC (permalink / raw)
To: Japin Li <japinli@hotmail.com>; +Cc: pgsql-hackers; pgsql-hackers@lists.postgresql.org
On 2022/07/21 23:41, Japin Li wrote:
>
> On Thu, 21 Jul 2022 at 22:22, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> Hi,
>>
>> I found that fetch_more_data_begin() in postgres_fdw reports an error when PQsendQuery() returns the value less than 0 as follows though PQsendQuery() can return only 1 or 0. I think this is a bug. Attached is the patch that fixes this bug. This needs to be back-ported to v14 where async execution was supported in postgres_fdw.
>>
>> if (PQsendQuery(fsstate->conn, sql) < 0)
>> pgfdw_report_error(ERROR, NULL, fsstate->conn, false, fsstate->query);
>>
>> Regards,
>
> +1. However, I think check whether the result equals 0 or 1 might be better.
Maybe. I just used "if (!PQsendQuery())" style because it's used in postgres_fdw elsewhere.
> Anyway, the patch works correctly.
Thanks for the review! Pushed.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().
2022-07-21 14:22 postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Fujii Masao <masao.fujii@oss.nttdata.com>
2022-07-21 14:41 ` Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Japin Li <japinli@hotmail.com>
2022-07-22 03:07 ` Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2022-07-22 08:03 ` Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 5+ messages in thread
From: Etsuro Fujita @ 2022-07-22 08:03 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: Japin Li <japinli@hotmail.com>; pgsql-hackers; pgsql-hackers@lists.postgresql.org
On Fri, Jul 22, 2022 at 12:07 PM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
> Pushed.
This is my oversight in commit 27e1f1456. :-(
Thanks for the report and fix, Fujii-san!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().
2022-07-21 14:22 postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2022-07-21 15:22 ` Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 5+ messages in thread
From: Tom Lane @ 2022-07-21 15:22 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: pgsql-hackers
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> I found that fetch_more_data_begin() in postgres_fdw reports an error when PQsendQuery() returns the value less than 0 as follows though PQsendQuery() can return only 1 or 0. I think this is a bug. Attached is the patch that fixes this bug. This needs to be back-ported to v14 where async execution was supported in postgres_fdw.
Yup, clearly a thinko.
regards, tom lane
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2022-07-22 08:03 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21 14:22 postgres_fdw: Fix bug in checking of return value of PQsendQuery(). Fujii Masao <masao.fujii@oss.nttdata.com>
2022-07-21 14:41 ` Japin Li <japinli@hotmail.com>
2022-07-22 03:07 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2022-07-22 08:03 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2022-07-21 15:22 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox