Hi Jian,
Thank you for the three patches. I answer them in order.
0001 Use WindowAggState instead of WindowObject in rpr routines
0002 minor, using palloc0_array instead of palloc0
> Most of the row pattern code already works on WindowAggState, so
> refactoring makes the remaining rpr routines take WindowAggState
> instead of WindowObject as well. This looks more consistent.
I will take 0001 and 0002.
0003 Remove WindowAggState.nfaStateSize
> I always want to get rid of WindowAggState.nfaStateSize. It's ugly.
I would like to keep that field. The size depends only on the
pattern, so it is computed once, and I do not want to repeat the
same computation on a path this frequent just to save a field. To
me the field is the cleaner choice.
> nfa_state_clone->nfa_state_make is hot path, but
> ExecRPRStartContext->nfa_state_make is not.
>
> In nfa_state_clone->nfa_state_make:
> memset(state, 0, winstate->nfaStateSize)
> is waste cycle, because nfa_state_clone would unconditionally memcpy
> from (int32 *counts) later.
I agree with this part, and I will take it. I went one step
further. nfa_state_clone calls nfa_state_make, so I would
merge the two into one, nfa_state_make:
static RPRNFAState *nfa_state_make(WindowAggState *winstate,
const RPRNFAState *src);
When src is NULL, it makes a new state on element 0 with counts[]
cleared (ExecRPRStartContext). Otherwise it copies elemIdx,
isAbsorbable and counts[] from src as they are. Of the 7 callers,
only the ALT branch sets elemIdx and isAbsorbable itself afterwards;
the others get them from nfa_state_exit_to() or stay on the same
element.
When Tatsuo prepares the next patch, I will make a patch for this
change on top of it and share it. I will send the accumulated
patches slowly, over time.
Best regards,
Henson
