Обсуждение: Use of uninitialized variables in ExecFindPartition() for parentpartition without leaves (HEAD only)

Hi all,

Since commit 4e5fe9ad (committer Robert Haas and author Amit Langote),
coverity has been complaining that the new code of ExecFindPartition()
may use a set of values and isnull values which never get initialized.
This is a state which can be easily reached with the following SQLs of
a parent partition with no children:
create table parent_tab (a int) partition by list ((a+0));
insert into parent_tab values (1);

The mistake is visibly that FormPartitionKeyDatum() should be called
even for a root partition, initializing the fields of isnull and
values on the way. That's actually what happens in execMain.c's
ExecFindPartition() if you look at REL_10_STABLE. So the answer would
be to move a bit down the quick exit code. Attached is a patch.

Amit, that's your code. What do you think?

Thanks,
-- 
Michael

Hi Michael.

On 2017/11/30 9:07, Michael Paquier wrote:
> Hi all,
> 
> Since commit 4e5fe9ad (committer Robert Haas and author Amit Langote),
> coverity has been complaining that the new code of ExecFindPartition()
> may use a set of values and isnull values which never get initialized.
> This is a state which can be easily reached with the following SQLs of
> a parent partition with no children:
> create table parent_tab (a int) partition by list ((a+0));
> insert into parent_tab values (1);
> 
> The mistake is visibly that FormPartitionKeyDatum() should be called
> even for a root partition, initializing the fields of isnull and
> values on the way. That's actually what happens in execMain.c's
> ExecFindPartition() if you look at REL_10_STABLE. So the answer would
> be to move a bit down the quick exit code. Attached is a patch.
> 
> Amit, that's your code. What do you think?

Thanks for the report.  That's clearly a bug. :-(

Your patch seems enough to fix it.  I added a test and expanded the
comment a bit in the attached updated version.

Thanks,
Amit

On 2017/11/30 10:11, Amit Langote wrote:
> in the attached updated version.

Oops, I messed up taking the diff and mistakenly added noise to the patch.
 Fixed in the attached.

Thanks,
Amit

On Thu, Nov 30, 2017 at 11:54 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> I have added a CF entry for this thread by the way
>> (https://commitfest.postgresql.org/16/1392/), and marked the thing as
>> ready for committer as we agree about the fix. Let's track properly
>> this issue until it gets committed.
>
> Yeah, thanks.

Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company