Hi David.
On 2018/05/14 13:57, David Rowley wrote:
> I noticed that a comment in get_partition_dispatch_recurse claims that:
>
> "it contains the
> * leaf partition's position in the global list *leaf_part_oids minus 1"
>
> The "minus 1" part is incorrect. It simply just stores the 0-based
> index of the item in the list.
Hmm, while I agree that simply calling it "0-based index" might be better
for readers, what's there now doesn't sound incorrect to me; in the
adjacent code:
if (get_rel_relkind(partrelid) != RELKIND_PARTITIONED_TABLE)
{
*leaf_part_oids = lappend_oid(*leaf_part_oids, partrelid);
pd->indexes[i] = list_length(*leaf_part_oids) - 1;
}
If I call the value of list_length after adding an OID to the list the
OID's position in the list, we're storing into the indexes array exactly
what the existing comment says it is. Now, literally describing the code
in the adjacent comment is not a great documentation style, so I'm open to
revising it like your patch does. :)
> I was going to fix it by removing just
> that part, but instead, I ended up rewriting the whole thing.
>
> Patch attached.
Looks good to me, thanks.
Regards,
Amit
