Re: negative bitmapset member not allowed Error with partition pruning
David Rowley <david.rowley@2ndquadrant.com>
From: David Rowley <david.rowley@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2018-07-27T04:34:40Z
Lists: pgsql-hackers
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
Commits
-
Verify range bounds to bms_add_range when necessary
- d25d45e4d922 12.0 landed
- a0655ba68fb5 11.0 landed
-
Change bms_add_range to be a no-op for empty ranges
- 1b68010518c9 12.0 landed
- 192d1bbf9969 11.0 landed