On 27 July 2018 at 15:14, Tom Lane <t...@sss.pgh.pa.us> wrote: > David Rowley <david.row...@2ndquadrant.com> writes: >> On 27 July 2018 at 13:35, Amit Langote <langote_amit...@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
