|From:||Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|Subject:||Re: Incorrect comments in partition.c|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
(2018/01/24 14:44), Etsuro Fujita wrote:
> (2018/01/24 13:06), Amit Langote wrote:
>> 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.
>> Makes sense.
>>> * 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 .
> Thanks for the review and related git info!
I'll add this to the upcoming commitfest.
|Next Message||Amit Langote||2018-02-28 08:36:56||inserts into partitioned table may cause crash|
|Previous Message||Etsuro Fujita||2018-02-28 08:22:42||Re: [HACKERS] Another oddity in handling of WCO constraints in postgres_fdw|