|From:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)lists(dot)postgresql(dot)org|
|Subject:||Re: Incorrect comments in partition.c|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2018/01/23 20:43, Etsuro Fujita wrote:
> Here is a comment for get_qual_for_list in partition.c:
> * get_qual_for_list
> * Returns an implicit-AND list of expressions to use as a list partition's
> - * constraint, given the partition key and bound structures.
> I don't think the part "given the partition key and bound structures."
> is correct because we pass the *parent relation* and partition bound
> structure to that function. So I think we should change that part as
> such. get_qual_for_range has the same issue, so I think we need this
> change for that function as well.
Yeah. It seems 6f6b99d1335  changed their signatures to support
handling default partitions.
> Another one I noticed in comments in partition.c is:
> * get_qual_for_hash
> * Given a list of partition columns, modulus and remainder
> corresponding to a
> * partition, this function returns CHECK constraint expression Node for
> * partition.
> I think the part "Given a list of partition columns, modulus and
> remainder corresponding to a partition" is wrong because we pass to that
> function the parent relation and partition bound structure the same way
> as for get_qual_for_list/get_qual_for_range. So what about changing the
> above to something like this, similarly to
> Returns a CHECK constraint expression to use as a hash partition's
> constraint, given the parent relation and partition bound structure.
> * The partition constraint for a hash partition is always a call to the
> * built-in function satisfies_hash_partition(). The first two
> arguments are
> * the modulus and remainder for the partition; the remaining arguments
> are the
> * values to be hashed.
> I also think the part "The first two arguments are the modulus and
> remainder for the partition;" is wrong (see satisfies_hash_partition).
> But I don't think we need to correct that here because we have described
> about the arguments in comments for that function. So I'd like to
> propose removing the latter comment entirely from the above.
Here, too. The comment seems to have been obsoleted by f3b0897a121 .
|Next Message||Tom Lane||2018-01-24 04:08:38||Re: [PATCH] fix for C4141 warning on MSVC|
|Previous Message||David Steele||2018-01-24 03:48:07||Re: PATCH: Configurable file mode mask|