| Lists: | pgsql-hackers |
|---|
| From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Igor Korot <ikorot01(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Improving overflow checks when adding tuple to PGresult Re: [GENERAL] Retrieving query results |
| Date: | 2017-08-29 01:30:55 |
| Message-ID: | CAB7nPqTnA3No2_B6x-QXPLHga1HatPqmXd03Xub+rPpshpmyMA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On Mon, Aug 28, 2017 at 3:05 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> Attached are two patches:
> 1) 0001 refactors the code around pqAddTuple to be able to handle
> error messages and assign them in PQsetvalue particularly.
> 2) 0002 adds sanity checks in pqAddTuple for overflows, maximizing the
> size of what is allocated to INT_MAX but now more.
>
> pqRowProcessor() still has errmsgp, but it is never used on HEAD. At
> least with this set of patches it comes to be useful. We could rework
> check_field_number() to use as well an error message string, but I
> have left that out to keep things simple. Not sure if any complication
> is worth compared to just copying the error message in case of an
> unmatching column number.
As this change requires I think an extra lookup, I am moving the
discussion to -hackers with a proper subject and the set of patches
attached (and the test program). This patch is registered in the next
commit fest.
--
Michael
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Refactor-error-message-handling-in-pqAddTuple.patch | application/octet-stream | 4.9 KB |
| 0002-Improve-overflow-checks-of-pqAddTuple-in-libpq.patch | application/octet-stream | 2.1 KB |
| pg_copy_res.c | text/x-csrc | 1.1 KB |
| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
| Cc: | Igor Korot <ikorot01(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Improving overflow checks when adding tuple to PGresult Re: [GENERAL] Retrieving query results |
| Date: | 2017-08-29 19:24:46 |
| Message-ID: | [email protected] |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> On Mon, Aug 28, 2017 at 3:05 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> Attached are two patches:
>> 1) 0001 refactors the code around pqAddTuple to be able to handle
>> error messages and assign them in PQsetvalue particularly.
>> 2) 0002 adds sanity checks in pqAddTuple for overflows, maximizing the
>> size of what is allocated to INT_MAX but now more.
I've pushed these (as one commit) with some adjustments. Mainly,
I changed PQsetvalue to report failure messages with PQinternalNotice,
which is what already happens inside check_field_number() for the case
of an out-of-range field number. It's possible that storing the
message into the PGresult in addition would be worth doing, but I'm
unconvinced about that --- we certainly haven't had any field requests
for it.
regards, tom lane
| From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Igor Korot <ikorot01(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Improving overflow checks when adding tuple to PGresult Re: [GENERAL] Retrieving query results |
| Date: | 2017-08-29 20:14:33 |
| Message-ID: | CAB7nPqSqU9dg0-FZoKm9kGZmU2bRFeDg3BzZCF0bLvXiQ0HgBw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On Wed, Aug 30, 2017 at 4:24 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>> On Mon, Aug 28, 2017 at 3:05 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> Attached are two patches:
>>> 1) 0001 refactors the code around pqAddTuple to be able to handle
>>> error messages and assign them in PQsetvalue particularly.
>>> 2) 0002 adds sanity checks in pqAddTuple for overflows, maximizing the
>>> size of what is allocated to INT_MAX but now more.
>
> I've pushed these (as one commit) with some adjustments.
Thanks!
> Mainly,
> I changed PQsetvalue to report failure messages with PQinternalNotice,
> which is what already happens inside check_field_number() for the case
> of an out-of-range field number. It's possible that storing the
> message into the PGresult in addition would be worth doing, but I'm
> unconvinced about that --- we certainly haven't had any field requests
> for it.
OK, fine for me.
--
Michael