Re: GSOC13 proposal - extend RETURNING syntax

Hi,

2013-08-19 21:52 keltezéssel, Boszormenyi Zoltan írta:
> 2013-08-19 21:21 keltezéssel, Karol Trzcionka írta:
>> W dniu 19.08.2013 19:56, Boszormenyi Zoltan pisze:
>>> * Does it apply cleanly to the current git master?
>>>
>>> No. There's a reject in src/backend/optimizer/plan/initsplan.c
>> Thank you, merged in attached version.
>>> * Does it include reasonable tests?
>>>
>>> Yes but the test fails after trying to fix the rejected chunk of the
>>> patch.
>> It fails because the "HINT" was changed, fixed.
>> That version merges some nested "ifs" left over from earlier work.
>
> I tried to compile your v5 patch and I got:
>
> initsplan.c: In function ‘add_vars_to_targetlist’:
> initsplan.c:216:26: warning: ‘rel’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
> rel->reltargetlist = lappend(rel->reltargetlist,
> ^
>
> You shouldn't change the assignment at declaration:
>
> - RelOptInfo *rel = find_base_rel(root, var->varno);
> + RelOptInfo *rel;
> ...
> + if (root->parse->commandType == CMD_UPDATE)
> + {
> ... (code using rel)
> + }
> + rel = find_base_rel(root, varno);

Let me say it again: the new code in initsplan.c::add_vars_to_targetlist() is fishy.
The compiler says that "rel" used on line 216 may be uninitialized.

Keeping it that way passes "make check", perhaps "rel" was initialized
in a previous iteration of "foreach(temp, vars)", possibly in the
     else if (IsA(node, PlaceHolderVar))
branch, which means that "PlaceHolderInfo *phinfo" may be interpreted
as RelOptInfo *, stomping on memory.

Moving the assignment back to the declaration makes "make check"
fail with the attached regression.diffs file.

Initializing it as "RelOptInfo *rel = NULL;" makes the regression check
die with a segfault, obviously.

Change the code to avoid the warning and still produce the wanted effect.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
      http://www.postgresql.at/