Re: [PATCH] Tiny optmization. - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [PATCH] Tiny optmization.
Date
Msg-id 20191122221855.2hidaegtejondhn3@development
Whole thread Raw
In response to RE: [PATCH] Tiny optmization.  (Ranier Vilela <ranier_gyn@hotmail.com>)
List 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.vondra@2ndquadrant.com>
>Enviado: sexta-feira, 22 de novembro de 2019 22:05
>Para: Ranier Vilela
>Cc: pgsql-hackers@postgresql.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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [PATCH] Tiny optmization.
Next
From: Ranier Vilela
Date:
Subject: RE: [PATCH] Tiny optmization.