On 27 July 2018 at 15:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> On 27 July 2018 at 13:35, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> On 2018/07/27 1:28, Tom Lane wrote:
>>>> (BTW, I'm not sure that it was wise to design bms_add_range to fail for
>>>> empty ranges. Maybe it'd be better to redefine it as a no-op for
>>>> upper < lower?)
>
>>> FWIW, I was thankful that David those left those checks there, because it
>>> helped expose quite a few bugs when writing this code or perhaps that was
>>> his intention to begin with, but maybe he thinks differently now (?).
>
>> I think it's more useful to keep as a bug catcher, although I do
>> understand the thinking behind just having it be a no-op.
>
> Well, my thinking is that it helps nobody if call sites have to have
> explicit workarounds for a totally-arbitrary refusal to handle edge
> cases in the primitive functions. I do not think that is good software
> design. If you want to have assertions that particular call sites are
> specifying nonempty ranges, put those in the call sites where it's
> important. But as-is, this seems like, say, defining foreach() to
> blow up on an empty list.
Okay, that's a fair point. I agree, adding Asserts at the current
call sites seems better.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services