On 03/21/2014 10:47 PM, Andres Freund wrote:
> On 2014-03-21 17:37:35 -0400, Tom Lane wrote:
>> Andres Freund <andres@2ndquadrant.com> writes:
>>> I think the GinLogicValueEnum is supposed to be an enum's name, not a
>>> variable name, right?
>>
>> I think the whole thing is too cute by half. Why isn't it just
>>
>> typedef enum GinLogicValue
>> {
>> GIN_FALSE = 0, /* item is present / matches */
>> GIN_TRUE = 1, /* item is not present / does not match */
>> GIN_MAYBE = 2 /* don't know if item is present / don't know if
>> * matches */
>> } GinLogicValue;
>>
>> instead of thinking that we are smarter than the compiler about how
>> to store the enum?
>
> It seems to be a memory only type, so using anything but the raw enum
> type seems odd. If it were ondisk alignment stuff could make it
> advantageous, but this way...
That enum is used in the "check" arrays that are passed around GIN, with
one element per index term being searched. I'd really like to keep it
small, because the can be hundreds of elements long, and if the compiler
decides to store it as a 4- or 8-byte value instead of one byte, it
starts to add up.
Besides memory usage, it's convenient that an array of GinLogicValues is
compatible with an array of booleans, as long as there are no GIN_MAYBE
values in it.
I committed your original fix to make it an enum type, like it was
supposed to be. Thanks!
- Heikki