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);
}