On Thu, Jun 17, 2021 at 4:18 PM Zhihong Yu <zyu@yugabyte.com> wrote:
> On Wed, Jun 16, 2021 at 10:37 PM Amit Langote <amitlangote09@gmail.com> wrote:
>> > + if (part_index < 0)
>> > + {
>> > + bound_offset = partition_range_datum_bsearch(key->partsupfunc,
>> >
>> > Do we need to check the value of equal before computing part_index ?
>>
>> Just in case you didn't notice, this is not new code, but appears as a
>> diff hunk due to indenting.
>>
>> As for whether the code should be checking 'equal', I don't think the
>> logic at this particular site should do that. Requiring 'equal' to be
>> true would mean that this code would only accept tuples that exactly
>> match the bound that partition_range_datum_bsearch() returned.
>
> Hi, Amit:
> Thanks for the quick response.
> w.r.t. the last point, since variable equal is defined within the case of PARTITION_STRATEGY_RANGE,
> I wonder if it can be named don_t_care or something like that.
> That way, it would be clearer to the reader that its value is purposefully not checked.
Normally, we write a comment in such cases, like
/* The value returned in 'equal' is ignored! */
Though I forgot to do that when I first wrote this code. :(
> It is fine to leave the variable as is since this was existing code.
Yeah, maybe there's not much to be gained by doing something about
that now, unless of course a committer insists that we do.
--
Amit Langote
EDB: http://www.enterprisedb.com