On 17 May 2018 at 02:51, Robert Haas <robertmh...@gmail.com> wrote: > On Mon, May 14, 2018 at 12:57 AM, David Rowley > <david.row...@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