Re: PQexec() hangs on OOM - Mailing list pgsql-bugs

On Mon, Dec 14, 2015 at 10:41 PM, Heikki Linnakangas <hlinnaka@iki.fi>
wrote:

> On 16/10/15 05:00, Michael Paquier wrote:
>
>> On Thu, Oct 15, 2015 at 11:28 PM, Amit Kapila <amit.kapila16@gmail.com>
>> wrote:
>>
>>> On Sun, Oct 11, 2015 at 6:31 PM, Michael Paquier <
>>> michael.paquier@gmail.com>
>>> wrote:
>>>
>>>>
>>>> Yeah, this behavior is caused by this piece of code:
>>>> @@ -600,7 +601,9 @@ getRowDescriptions(PGconn *conn, int msgLength)
>>>>   advance_and_error:
>>>>          /* Discard unsaved result, if any */
>>>>          if (result && result != conn->result)
>>>>                  PQclear(result);
>>>> An idea may be to check for conn->result->resultStatus !=
>>>> PGRES_FATAL_ERROR to concatenate correctly the error messages using
>>>> pqCatenateResultError. But just returning the first error encountered
>>>> as you mention would be more natural. So I updated the patch this way.
>>>>
>>>
> I pushed the ParamDescription fix. I'm not quite sure on the details of
> the COPY patch. Do you remember what this change in PQexecFinish is for:
>
> @@ -1991,6 +1995,9 @@ PQexecFinish(PGconn *conn)
>>          * We have to stop if we see copy in/out/both, however. We will
>> resume
>>          * parsing after application performs the data transfer.
>>          *
>> +        * Stop if we are in copy mode and error has occurred, the
>> pending results
>> +        * will be discarded during next execution in PQexecStart.
>> +        *
>>          * Also stop if the connection is lost (else we'll loop
>> infinitely).
>>          */
>>         lastResult = NULL;
>> @@ -2020,6 +2027,11 @@ PQexecFinish(PGconn *conn)
>>                         result->resultStatus == PGRES_COPY_BOTH ||
>>                         conn->status == CONNECTION_BAD)
>>                         break;
>> +               else if ((conn->asyncStatus == PGASYNC_COPY_IN ||
>> +                                 conn->asyncStatus == PGASYNC_COPY_OUT
>> ||
>> +                                 conn->asyncStatus == PGASYNC_COPY_BOTH)
>> &&
>> +                                result->resultStatus ==
>> PGRES_FATAL_ERROR)
>> +                       break;
>>         }
>>
>>         return lastResult;
>>
>
> I don't immediately see why that's needed. I ran the little tester program
> I wrote earlier (
> http://www.postgresql.org/message-id/55FC501A.5000409@iki.fi), with and
> without that change, and it behaved the same.
>
>
Without this change, it can ignore the OOM error for copy command.
As an example, for command like "copy t1 from stdin;", when the
flow reaches getCopyStart() in debugger, I have manually ensured
that PQmakeEmptyPGresult() return NULL by overwriting the return
value of malloc to zero and then it enters the error path
(advance_and_error) and then I just press continue and what I observed
is without above change it just doesn't show OOM error and with the
above change it properly reports OOM error.



With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com