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!) > > 0008: > > > > + xact_desc_stats(buf, "", parsed.nstats, parsed.stats); > > + xact_desc_stats(buf, "commit ", parsed.nstats, parsed.stats); > > + xact_desc_stats(buf, "abort ", parsed.nabortstats, parsed.abortstats); > > > > I'm not sure I like this, but I don't object to this.. > > The string prefixes? Or the entire patch? The string prefixes, since they are a limited set of fixed strings. That being said, I don't think it's better to use an enum instead, too. So I don't object to pass the strings here. > > 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) regards. -- Kyotaro Horiguchi NTT Open Source Software Center
