|From:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>|
|Subject:||Re: Incorrect comment in get_partition_dispatch_recurse|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
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.
|Next Message||Michael Paquier||2018-05-14 05:49:55||Temporary WAL segments files not cleaned up after an instance crash|
|Previous Message||Chapman Flack||2018-05-14 05:12:47||SPI/backend equivalent of extended-query Describe(statement)?|