David Geier <geidav...@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