Hi Henson, > Hi Jian, Tatsuo, > > Jian, two questions of yours from earlier in the thread never got an > answer from me. Late, but here are both. > >> Except that i want to rename RPSkipTo to RPRSkipTo and rpSkipTo to >> rprSkipTo, What do you think?
I am not sure what we are talking about because RPSkipTo is a typedef name but rpSkipTo is a struct member name. Do we want to establish a policy not only for typedef but for struct members? > Rather than a yes or no on this one name, I would like to treat it as > a policy call over the whole list. To be honest I don't have strong preference about struct or enum names. However, I suggest to decide a priority for this renaming work first. I think this kind work is best done after we stabilize our code. Because we can detect mistakes in the renaming work easier after establishing code rather than before. What do you think? BTW, when do we want to refactor WindowAggState so that RPR related members are grouped into a sub struct? I think this needed to be done while making our code commitable. > The split we have is historical. RP -- RPCommonSyntax, RPSkipTo -- > predates my joining this patch. RPR came later, from the pattern > compiler and the NFA I wrote. Neither author was aware of the other's > prefix; each of us just used his own. Now the two meet in one place, > since RPCommonSyntax holds an RPRPatternNode *rpPattern. > > The full list as it stands: > > RP* (parse / plan / exec nodes) > RPCommonSyntax, RPSkipTo > rpCommonSyntax, rpSkipTo, rpPattern, rpDefs (parsenodes.h) > rpSkipTo, rpPattern (plannodes.h, execnodes.h) > > RPR* (pattern compiler, NFA, navigation) > RPRPatternNode, RPRPatternNodeType, RPRPattern, > RPRPatternElement, RPRElemFlags, RPRElemIdx, RPRVarId, > RPRQuantity, RPRDepth, RPRNFAState, RPRNFAContext, > RPRNavExpr, RPRNavKind > RPR_ELEM_*, RPR_VARID_*, RPR_NAV_*, RPR_QUANTITY_INF, > RPR_VARID_MAX > > typedefs.list carries 15 of these -- 2 RP, 13 RPR. > > So what we would be choosing between is three policies: > > (a) RPR* throughout -- one prefix for the feature; the RP names > (7 identifiers, plus 2 typedefs.list entries) move over. > > (b) RP* throughout -- the other direction. It moves far more (13 > types, plus the whole RPR_ELEM_* / RPR_VARID_* / RPR_NAV_* > macro families) and cuts against everything already settled on > RPR: the file names (rpr.c, rpr.h, execRPR.c, README.rpr), the > tests (rpr.sql, rpr_explain.sql), and the acronym of the > feature itself (row pattern recognition). > > (c) RP for the row pattern syntax as the standard names it (row > pattern common syntax, AFTER MATCH SKIP TO), RPR for the > machinery that recognizes it -- a deliberate boundary, written > down so it reads as intent rather than as an accident. > > I have no strong preference between (a) and (c). (b) I list for > completeness -- with everything around it already RPR, it looks like > the most expensive of the three. Either way, what I am against is > settling this one name at a time. As I said earlier, I have no strong preference. So (a) is fine for me. > On timing, too: I would rather we decided it after the correctness > defects are closed. A rename touches the parse, plan and executor > nodes at once, so it collides with every patch in flight and with the > defect work under way -- and it buys a reader nothing today. Once the > defects are settled it is a single mechanical patch, and a much easier > review. Agreed. > Tatsuo, since RP is yours and RPR is mine, this is really a call for > you to make. Either way suits me. > > Jian, while I am here: v50-0003 (EMPTY_LOOP -> RPR_ELEM_EMPTY_LOOP) is > taken. Regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
