On 17 May 2018 at 02:51, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, May 14, 2018 at 12:57 AM, David Rowley
> <david.rowley@2ndquadrant.com> wrote:
>> The "minus 1" part is incorrect. It simply just stores the 0-based
>> index of the item in the list. I was going to fix it by removing just
>> that part, but instead, I ended up rewriting the whole thing.
>
> I think that's clearer. Committed with a few tweaks that are
> hopefully improvements.
Thanks for committing. Although, I disagree with your tweak:
+ * 1-based index into the *pds list.
I think that's making the same mistake as the last comment did. You
think it's 1-based because the index is being set with list_length
rather than list_length - 1, but it can do that simply because the
item has not been added to the list yet.
Nothing converts this index back to 0-based;
RelationGetPartitionDispatchInfo builds the array from the list with:
i = 0;
foreach(lc, pdlist)
{
pd[i++] = lfirst(lc);
}
ExecFindPartition uses the pd array with:
parent = pd[-parent->indexes[cur_index]];
So if it was 1-based then we'd be off by one here.
Maybe we can clear up that confusion with
+ /*
+ * No need to subtract 1 to get the 0-based index as the item for this
+ * partitioned table has not been added to the list yet.
+ */
pd->indexes[i] = -list_length(*pds);
and just switch 1-based to 0-based in the new comment.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
