On Thu, Jun 17, 2021 at 4:18 PM Zhihong Yu <z...@yugabyte.com> wrote: > On Wed, Jun 16, 2021 at 10:37 PM Amit Langote <amitlangot...@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