On Thu, Jan 27, 2022 at 4:59 PM Peter Smith <smithpb2...@gmail.com> wrote: > > On Thu, Jan 27, 2022 at 9:40 AM Greg Nancarrow <gregn4...@gmail.com> wrote: > > > > On Wed, Jan 26, 2022 at 2:08 PM houzj.f...@fujitsu.com > > <houzj.f...@fujitsu.com> wrote: > > > > > > There was a miss in the posted patch which didn't initialize the parameter in > > > RelationBuildPublicationDesc, sorry for that. Attach the correct patch this time. > > > > > > > A few comments for the v71-0001 patch: > ... > > (2) check_simple_rowfilter_expr_walker > > > > In the function header: > > (i) "etc" should be "etc." > > (ii) > > Is > > > > + * - (Var Op Const) Bool (Var Op Const) > > > > meant to be: > > > > + * - (Var Op Const) Logical-Op (Var Op Const) > > > > ? > > > > It's not clear what "Bool" means here. > > The comment is only intended as a generic example of the kinds of > acceptable expression format. > > The names in the comment used are roughly equivalent to the Node* tag names. > > This particular example is for an expression with AND/OR/NOT, which is > handled by a BoolExpr. > > There is no such animal as LogicalOp, so rather than change like your > suggestion I feel if this comment is going to change then it would be > better to change to be "boolop" (because the BoolExpr struct has a > boolop member). e.g. > > BEFORE > + * - (Var Op Const) Bool (Var Op Const) > AFTER > + * - (Var Op Const) boolop (Var Op Const) >
My use of "LogicalOp" was just indicating that the use of "Bool" in that line was probably meant to mean "Logical Operator", and these are documented in "9.1 Logical Operators" here: https://www.postgresql.org/docs/14/functions-logical.html (PostgreSQL docs don't refer to AND/OR etc. as boolean operators) Perhaps, to make it clear, the change for the example compound expression could simply be: + * - (Var Op Const) AND/OR (Var Op Const) or at least say something like " - where boolop is AND/OR". Regards, Greg Nancarrow Fujitsu Australia
