Re: [PATCH] Tiny optmization.

On Fri, Nov 22, 2019 at 10:24:05PM +0000, Ranier Vilela wrote:
>Hi,
>pstate is touched here:
>a) BeginCopy line 1489:
>    ProcessCopyOptions(pstate, cstate, is_from, options);
>b) ProcessCopyOptions line 1137:
>
>            if (format_specified)
>                ereport(ERROR,
>                        (errcode(ERRCODE_SYNTAX_ERROR),
>                         errmsg("conflicting or redundant options"),
>                         parser_errposition(pstate, defel->location)));
>

And? The fact that you pass a (possibly NULL) pointer somewhere does not
make that a dereference. And parser_errposition() does this:

    if (pstate == NULL || pstate->p_sourcetext == NULL)
        return 0;

So I fail to see why this would be an issue?



>best regards.
>Ranier Vilela
>
>________________________________________
>De: Tom Lane <tgl@sss.pgh.pa.us>
>Enviado: sexta-feira, 22 de novembro de 2019 22:17
>Para: Ranier Vilela
>Cc: pgsql-hackers@postgresql.org
>Assunto: Re: [PATCH] Tiny optmization.
>
>Ranier Vilela <ranier_gyn@hotmail.com> writes:
>> Redudant because he it's been dereferenced here:
>> line 3410:
>>         cstate = BeginCopy(pstate, true, rel, NULL, InvalidOid, attnamelist, options);
>
>Not necessarily ... the rel!=NULL code path there doesn't touch pstate,
>and that seems to be what contrib/file_fdw is relying on.
>
>Arguably, the rel==NULL code path in BeginCopy should be prepared to
>support pstate being null, too.  But what you proposed here is certainly
>not OK.
>
>                        regards, tom lane
>
>

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