On 6/7/2023 03:06, Alena Rybakina wrote:
The test was performed on the same benchmark database generated by 2 billion values.

I corrected this constant in the patch.
In attempt to resolve some issues had mentioned in my previous letter I used op_mergejoinable to detect mergejoinability of a clause. Constant side of the expression is detected by call of eval_const_expressions() and check each side on the Const type of node.

See 'diff to diff' in attachment.

--
regards,
Andrey Lepikhov
Postgres Professional
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 26648b0876..67d6f85010 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -111,38 +111,27 @@ transformBoolExprOr(ParseState *pstate, BoolExpr 
*expr_orig)
 {
        List               *or_list = NIL;
        List               *groups_list = NIL;
-       ListCell           *lc_eargs;
+       ListCell           *lc;
        Node               *result;
        BoolExpr           *expr;
        bool                    change_apply = false;
 
-       /* If this is not expression "Or", then will do it the old way. */
+       /* If this is not an 'OR' expression, skip the transformation */
        if (expr_orig->boolop != OR_EXPR ||
                list_length(expr_orig->args) < const_transform_or_limit)
-               return transformBoolExpr(pstate, (BoolExpr *)expr_orig);
+               return transformBoolExpr(pstate, (BoolExpr *) expr_orig);
 
        expr = (BoolExpr *)copyObject(expr_orig);
 
-       /*
-               * NOTE:
-               * It is an OR-clause. So, rinfo->orclause is a BoolExpr node, 
contains
-               * a list of sub-restrictinfo args, and rinfo->clause - which is 
the
-               * same expression, made from bare clauses. To not break 
selectivity
-               * caches and other optimizations, use both:
-               * - use rinfos from orclause if no transformation needed
-               * - use  bare quals from rinfo->clause in the case of 
transformation,
-               * to create new RestrictInfo: in this case we have no options 
to avoid
-               * selectivity estimation procedure.
-               */
-       foreach(lc_eargs, expr->args)
+       foreach(lc, expr->args)
        {
-               A_Expr                     *arg = (A_Expr *) lfirst(lc_eargs);
-               Node                       *bare_orarg;
+               Node                       *arg = lfirst(lc);
+               Node                       *orqual;
                Node                       *const_expr;
-               Node                       *non_const_expr;
+               Node                       *nconst_expr;
                ListCell                   *lc_groups;
                OrClauseGroupEntry *gentry;
-               bool                            allow_transformation;
+               bool                            const_is_left = true;
 
                /*
                 * The first step: checking that the expression consists only 
of equality.
@@ -154,33 +143,51 @@ transformBoolExprOr(ParseState *pstate, BoolExpr 
*expr_orig)
                 * same level of a single BoolExpr expression, otherwise all of 
them cannot be converted.
                 */
 
-               if (!arg)
-                       break;
+               orqual = transformExprRecurse(pstate, (Node *) arg);
+               orqual = coerce_to_boolean(pstate, orqual, "OR");
+               orqual = eval_const_expressions(NULL, orqual);
 
-               allow_transformation = (arg->type == T_A_Expr && (arg)->kind == 
AEXPR_OP &&
-                                                           
list_length((arg)->name) >=1 && strcmp(strVal(linitial((arg)->name)), "=") == 0
-                                                          );
+               if (!IsA(orqual, OpExpr))
+               {
+                       or_list = lappend(or_list, orqual);
+                       continue;
+               }
 
+               /* Detect constant side of the clause */
+               if (IsA(get_leftop(orqual), Const))
+                       const_is_left = true;
+               else if (IsA(get_rightop(orqual), Const))
+                       const_is_left = false;
+               else
+               {
+                       or_list = lappend(or_list, orqual);
+                       continue;
+               }
 
-               bare_orarg = transformExprRecurse(pstate, (Node *)arg);
-               bare_orarg = coerce_to_boolean(pstate, bare_orarg, "OR");
+               /* Get pointers to constant and expression sides of the qual */
+               nconst_expr = (const_is_left) ? get_rightop(orqual) :
+                                                                               
get_leftop(orqual);
+               const_expr = (const_is_left) ?  get_leftop(orqual) :
+                                                                               
get_rightop(orqual);
+
+               if (!op_mergejoinable(((OpExpr *) orqual)->opno, 
exprType(nconst_expr)))
+               {
+                       or_list = lappend(or_list, orqual);
+                       continue;
+               }
 
                /*
                 * The next step: transform all the inputs, and detect whether 
any contain
                 * Vars.
                 */
-               if (!allow_transformation || !bare_orarg || !IsA(bare_orarg, 
OpExpr) || !contain_vars_of_level(bare_orarg, 0))
+               if (!contain_vars_of_level(orqual, 0))
                {
                        /* Again, it's not the expr we can transform */
-                       or_list = lappend(or_list, bare_orarg);
+                       or_list = lappend(or_list, orqual);
                        continue;
                }
 
-               /*
-                * Get pointers to constant and expression sides of the clause
-                */
-               non_const_expr = get_leftop(bare_orarg);
-               const_expr = get_rightop(bare_orarg);
+               ;
 
                /*
                        * At this point we definitely have a transformable 
clause.
@@ -196,15 +203,15 @@ transformBoolExprOr(ParseState *pstate, BoolExpr 
*expr_orig)
 
                        Assert(v->node != NULL);
 
-                       if (equal(v->node, non_const_expr))
+                       if (equal(v->node, nconst_expr))
                        {
                                v->consts = lappend(v->consts, const_expr);
-                               non_const_expr = NULL;
+                               nconst_expr = NULL;
                                break;
                        }
                }
 
-               if (non_const_expr == NULL)
+               if (nconst_expr == NULL)
                        /*
                                * The clause classified successfully and added 
into existed
                                * clause group.
@@ -213,10 +220,10 @@ transformBoolExprOr(ParseState *pstate, BoolExpr 
*expr_orig)
 
                /* New clause group needed */
                gentry = palloc(sizeof(OrClauseGroupEntry));
-               gentry->node = non_const_expr;
+               gentry->node = nconst_expr;
                gentry->consts = list_make1(const_expr);
-               gentry->opno = ((OpExpr *)bare_orarg)->opno;
-               gentry->expr = (Expr *)bare_orarg;
+               gentry->opno = ((OpExpr *)orqual)->opno;
+               gentry->expr = (Expr *)orqual;
                groups_list = lappend(groups_list,  (void *) gentry);
        }
 

Reply via email to