Re: [BUG] Skipped initialization of some xl_xact_parsed_prepare fields

On 2025/05/15 14:39, Daniil Davydov wrote:
> Hi,
> I noticed that inside ParsePrepareRecord function we are missing
> initialization of `nstats` and `nabortstats` fields of
> xl_xact_parsed_prepare structure:
> *** (current code in master)
> memset(parsed, 0, sizeof(*parsed));
>
> parsed->xact_time = xlrec->prepared_at;
> parsed->origin_lsn = xlrec->origin_lsn;
> parsed->origin_timestamp = xlrec->origin_timestamp;
> parsed->twophase_xid = xlrec->xid;
> parsed->dbId = xlrec->database;
> parsed->nsubxacts = xlrec->nsubxacts;
> parsed->nrels = xlrec->ncommitrels;
> parsed->nabortrels = xlrec->nabortrels;
> parsed->nmsgs = xlrec->ninvalmsgs;
> ***
>
> Thus, they will always be 0 after calling the ParsePrepareRecord
> function, but `stats` and `abortstats` pointers are set correctly:
> *** (current code in master)
> parsed->stats = (xl_xact_stats_item *) bufptr;
> bufptr += MAXALIGN(xlrec->ncommitstats * sizeof(xl_xact_stats_item));
>
> parsed->abortstats = (xl_xact_stats_item *) bufptr;
> bufptr += MAXALIGN(xlrec->nabortstats * sizeof(xl_xact_stats_item));
> ***
>
> If we look (for example) on parsing of a commit record, we could see
> that both `nstats` and `stats` fields are set inside the
> ParseCommitRecord function.
> For now, zeroed number of stats lead to invalid output of
> `xact_desc_prepare`, because it relies on values of `nstats` and
> `nabortstats` fields:
> *** (current code in master)
> xact_desc_stats(buf, "commit ", parsed.nstats, parsed.stats);
> xact_desc_stats(buf, "abort ", parsed.nabortstats, parsed.abortstats);
> ***

Thanks for the report! You're right.

> I suppose to add initialization of `nstats` and `nabortstats` to
> ParsePrepareRecord (see attached patch).

LGTM. Barring any objection, I will commit this patch.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION