Re: negative bitmapset member not allowed Error with partitionpruning

On 2018/07/27 1:28, Tom Lane wrote:
> Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com> writes:
>> I am getting "ERROR:  negative bitmapset member not allowed" when
>> enable_partition_pruning set to true with below test case.

Thanks Rajkumar.

> Confirmed here.  It's failing in perform_pruning_combine_step,
> which reaches this:
> 
>         result->bound_offsets = bms_add_range(NULL, 0, boundinfo->ndatums - 1);
> 
> with boundinfo->ndatums == 0.  It's not clear to me whether that situation
> should be impossible or not.  If it is valid, perhaps all we need is
> something like
> 
>         if (boundinfo->ndatums > 0)
>             result->bound_offsets = bms_add_range(NULL, 0, boundinfo->ndatums - 1);
>         else
>             result->bound_offsets = NULL;
> 
> although that then opens the question of whether downstream code is
> OK with bound_offsets being empty.

Yeah, this seems to be the only possible fix and I checked that downstream
code is fine with bound_offsets being NULL/empty.  Actually, the code
that's concerned with bound offsets is limited to partprune.c, because we
don't propagate bound offsets themselves outside this file.

I found one more place in get_matching_hash_bounds where I thought maybe
it'd be a good idea to add this check (if ndatums > 0), but then realized
that that would become dead code as the upstream code takes care of the 0
hash partitions case.  So, maybe an Assert (ndatums > 0) would be better.

Attached find a patch that does both.

> (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 (?).

Thanks,
Amit