From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | melanieplageman(at)gmail(dot)com, pryzby(at)telsasoft(dot)com, thomas(dot)munro(at)gmail(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: shared-memory based stats collector - v70 |
Date: | 2022-04-07 01:58:52 |
Message-ID: | 20220407015852.7xehapb2z7iebqpt@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2022-04-07 10:36:30 +0900, Kyotaro Horiguchi wrote:
> At Wed, 6 Apr 2022 09:04:09 -0700, Andres Freund <andres(at)anarazel(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2022-04-07 02:36:37 | Re: Window Function "Run Conditions" |
Previous Message | Robert Haas | 2022-04-07 01:39:22 | Re: Should pg_dumpall dump ALTER SYSTEM settings? |