David Geier <geidav.pg@gmail.com> writes:
> On 11/18/22 14:00, Tomas Vondra wrote:
>> Seems fine. I wonder if we could/could introduce a new constant for 0,
>> similar to ATTSTATSSLOT_NUMBERS/ATTSTATSSLOT_VALUES, instead of using a
>> magic constant. Say, ATTSTATSSLOT_NONE or ATTSTATSSLOT_CHECK.
> Good idea. I called it ATTSTATSSLOT_EXISTS. New patch attached.
No, I don't think it's a good idea. The flags argument is documented as,
and used as, a bitmask of multiple options. Passing zero fits fine with
that and is consistent with what we do elsewhere. Turning it into
sort-of-an-enum-but-not-really isn't an improvement.
I didn't like your draft comment too much, because it didn't cover
what I think is the most important point: after a call with flags=0
we do not need a matching free_attstatsslot call to avoid leaking
anything. (If we did, this patch would be a lot hairier.)
I rewrote the comment the way I wanted it and pushed.
regards, tom lane