Horton

Thread

Commits

  1. postgres_fdw: Fix bug in checking of return value of PQsendQuery().

  2. Add support for asynchronous execution.

  1. postgres_fdw: Fix bug in checking of return value of PQsendQuery().

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2022-07-21T14:22:26Z

    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

  2. Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().

    Japin Li <japinli@hotmail.com> — 2022-07-21T14:41:54Z

    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.

  3. Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-21T15:22:48Z

    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

  4. Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2022-07-22T03:07:28Z

    
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

  5. Re: postgres_fdw: Fix bug in checking of return value of PQsendQuery().

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2022-07-22T08:03:19Z

    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