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 [1] 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 > that > * 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 > get_qual_for_list/get_qual_for_range: > > Returns a CHECK constraint expression to use as a hash partition's > constraint, given the parent relation and partition bound structure. Makes sense. > Also: > > * 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 [2]. Thanks, Amit [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6f6b99d1335 [2] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=f3b0897a121