Hi Henson, I looked into 0051 and have some suggestions.
If you like, can you please included the changes in v49? I am going to release v48 soon. Regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp > index 1a754bcdac5..7ba0b6df849 100644 > --- a/src/backend/commands/explain.c > +++ b/src/backend/commands/explain.c > +static int > +deparse_rpr_node(RPRPattern *pattern, int i, int limit, StringInfo buf) One letter function argument name "i" looks unusual and better to be named more meaningful one, like idx? > /* > - * Process an ALT element: adjust depth parens and register separator > positions. > + * Find the END that closes the group opened by the BEGIN at beginIdx: the > + * first END at the same depth scanning forward. > */ > -static void > -deparse_rpr_alt(RPRPattern *pattern, int *idx, StringInfoData *buf, > - RPRDepth *prevDepth, bool *needSpace, List > **altSeps) > +static int > +rpr_match_end(RPRPattern *pattern, int beginIdx) > { > - RPRPatternElement *elem = &pattern->elements[*idx]; > - > - /* Close parens for depth decrease */ > - while (*prevDepth > elem->depth) > - { > - appendStringInfoChar(buf, ')'); > - (*prevDepth)--; > - *needSpace = true; > - } > - > - /* Open parens up to ALT's depth */ > - while (*prevDepth < elem->depth) > - { > - if (*needSpace) > - appendStringInfoChar(buf, ' '); > - appendStringInfoChar(buf, '('); > - (*prevDepth)++; > - *needSpace = false; > - } > + RPRDepth d = pattern->elements[beginIdx].depth; > + int j; If we change function arugument to other than "i", "j" here better to be named "i". > /* > - * Process a VAR element: adjust depth parens and output variable name. > + * Scope end of the construct at index i: the first following element whose > + * depth is no greater than i's own. For an ALT marker this is the index > just > + * past its last branch, since depth stays constant across branch boundaries. > + * FIN sits at depth 0, so a top-level ALT stops there. > */ > -static void > -deparse_rpr_var(RPRPattern *pattern, int *idx, StringInfoData *buf, > - RPRDepth *prevDepth, bool *needSpace, List > **altSeps) > +static int > +rpr_alt_scope_end(RPRPattern *pattern, int i) One letter argument name "i" looks unusual and better to be named more meaningful one, like idx? > { > - RPRPatternElement *elem = &pattern->elements[*idx]; > - > - /* Open parens for depth increase */ > - while (*prevDepth < elem->depth) > - { > - if (*needSpace) > - appendStringInfoChar(buf, ' '); > - appendStringInfoChar(buf, '('); > - (*prevDepth)++; > - *needSpace = false; > - } > + RPRDepth d = pattern->elements[i].depth; > + int k; If we change function arugument to other than "i", "j" here better to be named "i". > +/* > + * Boundary of the alternation branch starting at b (i.e. the start of the > next > + * branch, or altEnd if b is the last branch). > + * > + * The branch-start element's jump points at the next branch when this is not > + * the last branch. jump is overloaded (a group BEGIN also uses it for its > + * skip path), so confirm a real branch boundary with the relative test > + * elem[j-1].next != j: at a true boundary the preceding branch's tail has > its > + * next redirected past the alternation, so it does not point at j. > + */ > +static int > +rpr_next_branch(RPRPattern *pattern, int b, int altEnd) One letter function argument name "b" looks unusual and better to be named more meaningful one, like bno or bid? Regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
