Re: [Bug fix] PQsendQuery occurs error when target_session_attrs is set to read-write

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: "Higuchi, Daisuke" <higuchi(dot)daisuke(at)jp(dot)fujitsu(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [Bug fix] PQsendQuery occurs error when target_session_attrs is set to read-write
Date: 2017-02-02 04:34:12
Message-ID: CAFjFpRfcLJ7yihFLHQLh4jDO4LKhUHMngDKz5Z=2+491Ponm=A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Feb 2, 2017 at 8:11 AM, Higuchi, Daisuke
<higuchi(dot)daisuke(at)jp(dot)fujitsu(dot)com> wrote:
> From: Ashutosh Bapat [mailto:ashutosh(dot)bapat(at)enterprisedb(dot)com]
>> Per the documentation [1], "PQgetResult must be called repeatedly
>> until it returns a null pointer, indicating that the command is
>> done.". The code in PQgetResult() under CONNECTION_CHECK_WRITABLE
>> case, violates that. The patch fixes it. The patch however jumps to
>> keep_going without changing conn->status, which means that it will end
>> up again in the same case. While that's fine, may be we should use a
>> local loop on PQgetResult() to keep the code readable.
> Thank you for reviewing the patch.
> I created it with reference to pqSetenvPoll() in interfaces/libpq/fe-protocol2.c,
> but I certainly thought that readability is not good.
> I updated the patch, so I will add this to the next commitfest.

Thanks for the patch.

The code expects that there will be two PQgetResult() calls required.
One to fetch the result of SHOW command and the other to extract NULL.
If we require more calls or unexpected results, we should throw and
error. The patch just checks the first result and consumes the
remaining without verifying them. Also, it looks like we can not clear
result of PQgetResult() before using the values or copying them
somewhere else [1]. Here's updated patch which tries to do that.
Please let me know if this looks good to you.

[1] https://www.postgresql.org/docs/devel/static/libpq-exec.html: PQgetvalue().
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Attachment Content-Type Size
PQsendQuery_for_target_session_attrs_v3.patch application/octet-stream 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2017-02-02 04:34:51 Re: [Bug fix] PQsendQuery occurs error when target_session_attrs is set to read-write
Previous Message Tom Lane 2017-02-02 04:27:36 Re: TRAP: FailedAssertion("!(hassrf)", File: "nodeProjectSet.c", Line: 180)