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.von...@2ndquadrant.com>
Enviado: sexta-feira, 22 de novembro de 2019 22:05
Para: Ranier Vilela
Cc: pgsql-hack...@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
