Re: [PATCH] Tiny optmization.

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Ranier Vilela <ranier_gyn(at)hotmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Tiny optmization.
Date: 2019-11-22 22:18:55
Message-ID: 20191122221855.2hidaegtejondhn3@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 22, 2019 at 10:10:29PM +0000, Ranier Vilela wrote:
>Hi,
>Redudant because he it's been dereferenced here:
>
>line 3410:
> cstate = BeginCopy(pstate, true, rel, NULL, InvalidOid, attnamelist, options);
>

There's no pstate dereference here. It just passed the value to
BeginCopy.

BTW please don't top post, reply inline. It's much easier to follow the
discussion.

>Best regards.
>Ranier Vilela
>
>________________________________________
>De: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
>Enviado: sexta-feira, 22 de novembro de 2019 22:05
>Para: Ranier Vilela
>Cc: pgsql-hackers(at)postgresql(dot)org
>Assunto: Re: [PATCH] Tiny optmization.
>
>On Fri, Nov 22, 2019 at 09:41:44PM +0000, Ranier Vilela wrote:
>>Hi,
>>Maybe it doesn't matter, but, I think it's worth discussing.
>>The expression "if(pstate)" is redundant or is possible null dereference.
>>
>
>Eh? Redundant with what? Why would it be a null dereference? It's a
>parameter passed from outside, and we're not checking it before. And
>the if condition is there exactly to prevent null dereference.
>
>It's generally a good idea to inspect existing callers of the modified
>function and try running tests before submitting a patch. In this case
>there's a BeginCopyFrom() call in contrib/file_fdw, passing NULL as the
>first parameter, and if you run `make check` for that module it falls
>flat on it's face due to a segfault.
>
>regards
>
>>
>>--- \dll\postgresql-12.0\a\backend\commands\copy.c Mon Sep 30 17:06:55 2019
>>+++ copy.c Fri Nov 22 18:33:05 2019
>>@@ -3426,8 +3426,7 @@
>> cstate->raw_buf_index = cstate->raw_buf_len = 0;
>>
>> /* Assign range table, we'll need it in CopyFrom. */
>>- if (pstate)
>>- cstate->range_table = pstate->p_rtable;
>>+ cstate->range_table = pstate->p_rtable;
>>
>> tupDesc = RelationGetDescr(cstate->rel);
>> num_phys_attrs = tupDesc->natts;
>
>
>
>--
>Tomas Vondra http://www.2ndQuadrant.com
>PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2019-11-22 22:24:05 RE: [PATCH] Tiny optmization.
Previous Message Tom Lane 2019-11-22 22:17:33 Re: [PATCH] Tiny optmization.