Hi, > An automatic claude review pointed out that a stats file hash entry > for a fixed-numbered kind now crashes during startup.
Good find. > The attached patch fixes this and includes a simple test case in it to > show the issue. A test is good here. I don't think we have enough test coverage for pgstat startup. However, I think we could keep the proposed test simpler, and flip the kind from variable to fixed once, which is enough. Also, despite the comments in pgstat_internal.h: ``` /* * For variable-numbered stats. Optional. */ void (*to_serialized_name) (const PgStat_HashKey *key, const PgStatShared_Common *header, NameData *name); bool (*from_serialized_name) (const NameData *name, PgStat_HashKey *key); ``` we do not prevent a fixed-numbered kind from defining these callbacks. I think we should harden that. With it in place, the new check is only needed for PGSTAT_FILE_ENTRY_HASH, as a name entry for a fixed-numbered kind will then always fail the !from_serialized_name check when PGSTAT_FILE_ENTRY_NAME. 0001 disallows the serialized name callbacks for fixed-numbered kinds. 0002 is your fix with the check in the hash entry branch and the simplified test. -- Sami
v1-0001-Disallow-serialized-name-callbacks-for-fixed-numb.patch
Description: Binary data
v1-0002-Fix-crash-at-startup-with-stats-file-entry-of-fix.patch
Description: Binary data
