Hi, On 2022-04-07 10:36:30 +0900, Kyotaro Horiguchi wrote: > At Wed, 6 Apr 2022 09:04:09 -0700, Andres Freund <and...@anarazel.de> wrote > in > > > + * Don't define an INVALID value so switch() statements can warn if some > > > + * cases aren't covered. But define the first member to 1 so that > > > + * uninitialized values can be detected more easily. > > > > > > FWIW, I like this. > > > > I think there's no switches left now, so it's not actually providing too > > much. > > (Ouch!)
I think it's great that there's no switches left - means we're pretty close to pgstat being runtime extensible... > > > 0010: > > > (I didn't look this closer. The comments arised while looking other > > > patches.) > > > > > > +pgstat_kind_from_str(char *kind_str) > > > > > > I don't think I like "str" so much. Don't we spell it as > > > "pgstat_kind_from_name"? > > > > name makes me think of NameData. What do you dislike about str? We seem to > > use > > str in plenty places? > > For clarity, I don't dislike it so much. So, I'm fine with the > current name. > > I found that you meant a type by the "str". I thought it as an > instance (I'm not sure I can express my feeling correctly here..) and > the following functions were in my mind. > > char *get_namespace/rel/collation/func_name(Oid someoid) > char *pgstat_slru_name(int slru_idx) > > Another instance of the same direction is > > ForkNumber forkname_to_number(const char *forkName) It's now pgstat_get_kind_from_str(). It was harder to see earlier (I certainly didn't really see it) - because there were so many "violations" - but most of pgstat is pgstat_<verb>_<subject>() or just <verb>_<subject>. I'd already moved most of the patch series over to that (maybe in v68 or so). Now I also did that with the internal functions. There's a few functions breaking that pattern, partially because I added them :(, but since they're not touched in these patches I've not renamed them. But it's probably worth doing so tomorrow. Greetings, Andres Freund