Hi Henson, > Therefore, I believe it only makes sense to add explicit initialization > for nfaStatesActive and nfaContextsActive, since these are the only two > that release_partition() also explicitly resets.
That makes sense. > Adding explicit > initialization for all the cumulative statistics would be misleading, > as it might suggest they get reset somewhere (which they don't). > > ## Proposal for consistency > > For consistency with release_partition(), I can add just the two > per-partition counter initializations to ExecInitWindowAgg(): > > /* Initialize NFA free lists for row pattern matching */ > winstate->nfaContext = NULL; > winstate->nfaContextTail = NULL; > winstate->nfaContextFree = NULL; > winstate->nfaStateFree = NULL; > winstate->nfaLastProcessedRow = -1; > winstate->nfaStatesActive = 0; // Add this > winstate->nfaContextsActive = 0; // Add this > > This would make both ExecInitWindowAgg() and release_partition() > initialize the same set of per-partition NFA fields, making the code > more uniform and easier to review. The cumulative statistics fields > remain omitted in both functions, which correctly reflects that they > are never reset. > > Functionally, this change makes no difference since palloc0() already > zeros these fields, but it improves code consistency and makes the > per-partition vs. query-wide distinction clearer. > > Would you like me to include this change in the next patch? Yes, please. Best regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
