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
diff --git a/src/backend/executor/execPartition.c
b/src/backend/executor/execPartition.c
index d275cefe1d..8b334ea2ff 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -206,13 +206,6 @@ ExecFindPartition(ResultRelInfo *resultRelInfo,
PartitionDispatch *pd,
slot = myslot;
}
- /* Quick exit */
- if (partdesc->nparts == 0)
- {
- result = -1;
- break;
- }
-
/*
* Extract partition key from tuple. Expression evaluation
machinery
* that FormPartitionKeyDatum() invokes expects ecxt_scantuple
to
@@ -223,6 +216,17 @@ ExecFindPartition(ResultRelInfo *resultRelInfo,
PartitionDispatch *pd,
*/
ecxt->ecxt_scantuple = slot;
FormPartitionKeyDatum(parent, slot, estate, values, isnull);
+
+ /*
+ * Nothing for get_partition_for_tuple() to do if there are no
+ * partitions to begin with.
+ */
+ if (partdesc->nparts == 0)
+ {
+ result = -1;
+ break;
+ }
+
cur_index = get_partition_for_tuple(rel, values, isnull);
/*
@@ -472,7 +476,7 @@ FormPartitionKeyDatum(PartitionDispatch pd,
}
/*
- * BuildSlotPartitionKeyDescription
+ * ExecBuildSlotPartitionKeyDescription
*
* This works very much like BuildIndexValueDescription() and is currently
* used for building error messages when ExecFindPartition() fails to find
diff --git a/src/test/regress/expected/insert.out
b/src/test/regress/expected/insert.out
index b7b37dbc39..dcbaad8e2f 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -165,6 +165,10 @@ create table range_parted (
a text,
b int
) partition by range (a, (b+0));
+-- no partitions, so fail
+insert into range_parted values ('a', 11);
+ERROR: no partition of relation "range_parted" found for row
+DETAIL: Partition key of the failing row contains (a, (b + 0)) = (a, 11).
create table part1 partition of range_parted for values from ('a', 1) to ('a',
10);
create table part2 partition of range_parted for values from ('a', 10) to
('a', 20);
create table part3 partition of range_parted for values from ('b', 1) to ('b',
10);
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 310b818076..0150b6bb0f 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -90,6 +90,10 @@ create table range_parted (
a text,
b int
) partition by range (a, (b+0));
+
+-- no partitions, so fail
+insert into range_parted values ('a', 11);
+
create table part1 partition of range_parted for values from ('a', 1) to ('a',
10);
create table part2 partition of range_parted for values from ('a', 10) to
('a', 20);
create table part3 partition of range_parted for values from ('b', 1) to ('b',
10);
