At Thu, 24 Mar 2022 13:21:33 -0400, Melanie Plageman 
<melanieplage...@gmail.com> wrote in 
> On Thu, Mar 17, 2022 at 3:36 AM Andres Freund <and...@anarazel.de> wrote:
> >
> > The biggest todos are:
> > - Address all the remaining AFIXMEs and XXXs
> 
> Attached is a patch that addresses three of the existing AFIXMEs.

Thanks!

+               .reset_timestamp_cb = pgstat_shared_reset_timestamp_noop,

(I once misunderstood that the "shared" means shared memory area..)

The reset function is type-specific and it must be set.  So don't we
provide all to-be-required reset functions?


+       if (pgstat_shared_ref_get(kind, dboid, objoid, false, NULL))
+       {
+               Oid msg_oid = (kind == PGSTAT_KIND_DB) ? dboid : objoid;

Explicitly using PGSTAT_KIND_DB here is a kind of annoyance.  Since we
always give InvalidOid correctly as the parameters, and objoid alone
is not specific enough, do we warn using both dboid and objoid without
a special treat?

Concretely, I propose to do the following instead.

+       if (pgstat_shared_ref_get(kind, dboid, objoid, false, NULL))
+       {
+               ereport(WARNING,
+                               errmsg("resetting existing stats for type %s, 
db=%d, oid=%d",
+                      pgstat_kind_info_for(kind)->name, dboid, objoid);        
                        



+pgstat_pending_delete(PgStatSharedRef *shared_ref)
+{
+       void       *pending_data = shared_ref->pending;
+       PgStatKind kind = shared_ref->shared_entry->key.kind;
+
+       Assert(pending_data != NULL);
+       Assert(!pgstat_kind_info_for(kind)->fixed_amount);
+
+       /* PGSTAT_KIND_TABLE has its own callback */
+       Assert(kind != PGSTAT_KIND_TABLE);
+

"kind" is used only in assertion, which requires PG_USED_FOR_ASSERTS_ONLY.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


Reply via email to