From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Sloppiness around failure handling of parsePGArray in pg_dump |
Date: | 2020-11-19 09:36:26 |
Message-ID: | 980DBA84-8288-4541-8E54-C10396B6D09C@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 19 Nov 2020, at 02:37, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Nov 18, 2020 at 10:19:40AM +0100, Daniel Gustafsson wrote:
>> I agree that we should fix this even if it will have quite limited impact in
>> production settings. Patch LGTM, +1.
>
> Thanks. I have reviewed that again this morning and applied it.
>
>> Another thing caught my eye here (while not the fault of this patch), we ensure
>> to clean up array leftover in case of parsePGArray failure, but we don't clean
>> up the potential allocations from the previous calls. Something like the below
>> seems more consistent.
>>
>> @@ -12105,6 +12099,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
>> nitems != nallargs)
>> {
>> pg_log_warning("could not parse proargmodes array");
>> + if (allargtypes)
>> + free(allargtypes);
>> if (argmodes)
>> free(argmodes);
>> argmodes = NULL;
>> @@ -12119,6 +12115,10 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
>> nitems != nallargs)
>> {
>> pg_log_warning("could not parse proargnames array");
>> + if (allargtypes)
>> + free(allargtypes);
>> + if (argmodes)
>> + free(argmodes);
>> if (argnames)
>> free(argnames);
>> argnames = NULL;
>
> If you do that, I think that's not completely correct either.
> format_function_arguments_old() has some logic to allow the process to
> go through for pre-8.4 dumps as long as allargtypes includes correct
Ah, yes, I read that wrong. It's correct as it is.
cheers ./daniel
From | Date | Subject | |
---|---|---|---|
Next Message | Dean Rasheed | 2020-11-19 09:52:04 | Re: Additional improvements to extended statistics |
Previous Message | japin | 2020-11-19 09:35:05 | Re: Terminate the idle sessions |