On 12/25/17 00:32, Michael Paquier wrote:
>> So here is a minimal patch set to perhaps wrap this up for the time
>> being. I have added static assertions that check the sizes of
>> GinNullCategory and GinTernaryValue, which I think are the two critical
>> places that require compatibility with bool. And then we include
>> <stdbool.h> only if its bool has size 1.
>
> + /*
> + * now we can use the nullFlags as category codes
> + */
> + StaticAssertStmt(sizeof(GinNullCategory) == sizeof(bool),
> + "sizes of GinNullCategory and bool are not equal");
> *categories = (GinNullCategory *) nullFlags;
>
> Hm. So on powerpc compilation is going to fail with this patch as
> sizeof(char) is 1, no?
Yes, but right now it would (probably) just fail in mysterious ways, so
the static assertion adds safety.
> Wouldn't it be better to just allocate an array
> for GinNullCategory entries and then just fill in the values one by
> one with GIN_CAT_NORM_KEY or GIN_CAT_NULL_KEY by scanning nullFlags?
Yeah, initially I though making another loop through the array would be
adding more overhead. But reading the code again, we already loop
through the array anyway to set the nullFlags to the right bit patterns.
So I think we can just drop that and build a proper GinNullCategory
array instead. I think that would be much cleaner. Patch attached.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services