On Tue, Dec 14, 2021 at 10:50 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Dec 14, 2021 at 4:44 AM Peter Smith <smithpb2...@gmail.com> wrote: > > Few other comments: > ===================
Few more comments: ================== v46-0001/0002 =============== 1. After rowfilter_walker() why do we need EXPR_KIND_PUBLICATION_WHERE? I thought this is primarily to identify the expressions that are not allowed in rowfilter which we are now able to detect upfront with the help of a walker. Can't we instead use EXPR_KIND_WHERE? 2. +Node * +GetTransformedWhereClause(ParseState *pstate, PublicationRelInfo *pri, + bool bfixupcollation) Can we add comments atop this function? 3. In GetTransformedWhereClause, can we change the name of variables (a) bfixupcollation to fixup_collation or assign_collation, (b) transformedwhereclause to whereclause. I think that will make the function more readable. v46-0002 ======== 4. + else if (IsA(node, List) || IsA(node, Const) || IsA(node, BoolExpr) || IsA(node, NullIfExpr) || + IsA(node, NullTest) || IsA(node, BooleanTest) || IsA(node, CoalesceExpr) || + IsA(node, CaseExpr) || IsA(node, CaseTestExpr) || IsA(node, MinMaxExpr) || + IsA(node, ArrayExpr) || IsA(node, ScalarArrayOpExpr) || IsA(node, XmlExpr)) Can we move this to a separate function say IsValidRowFilterExpr() or something on those lines and use Switch (nodetag(node)) to identify these nodes? -- With Regards, Amit Kapila.
