On Tue, Dec 14, 2021 at 10:12 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > 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? >
Fixed in v47 [1] > 2. > +Node * > +GetTransformedWhereClause(ParseState *pstate, PublicationRelInfo *pri, > + bool bfixupcollation) > > Can we add comments atop this function? > Fixed in v47 [1] > 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. > Fixed in v47 [1] > > 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? > Fixed in v47 [1] ------ [1] https://www.postgresql.org/message-id/CAHut%2BPtjsj_OVMWEdYp2Wq19%3DH5D4Vgta43FbFVDYr2LuS_djg%40mail.gmail.com Kind Regards, Peter Smith. Fujitsu Australia
