On 27 July 2018 at 15:14, Tom Lane <[email protected]> wrote: > David Rowley <[email protected]> writes: >> On 27 July 2018 at 13:35, Amit Langote <[email protected]> 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
