On 7/7/2023 15:20, Alena Rybakina wrote:
because we will provide similar manipulation in this:
foreach(l, gentry->consts)
{
Node *rexpr = (Node *) lfirst(l);
rexpr = coerce_to_common_type(pstate, rexpr,
scalar_type,
"IN");
aexprs = lappend(aexprs, rexpr);
}
I'm not sure that it should be replaced.
In attachment - a bit more corrections to the patch.
The most important change - or_list contains already transformed
expression subtree. So, I think we don't need to free it at all.
--
regards,
Andrey Lepikhov
Postgres Professional
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 961ca3e482..f0fd63f05c 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -112,9 +112,6 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
List *or_list = NIL;
List *groups_list = NIL;
ListCell *lc;
- Node *result;
- BoolExpr *expr;
- bool change_apply = false;
/* If this is not an 'OR' expression, skip the transformation */
if (expr_orig->boolop != OR_EXPR ||
@@ -131,16 +128,7 @@ transformBoolExprOr(ParseState *pstate, BoolExpr
*expr_orig)
OrClauseGroupEntry *gentry;
bool const_is_left = true;
- /*
- * The first step: checking that the expression consists only
of equality.
- * We can only do this here, while arg is still row data type,
namely A_Expr.
- * After applying transformExprRecurce, we already know that it
will be OpExr type,
- * but checking the expression for equality is already becoming
impossible for us.
- * Sometimes we have the chance to devide expression into the
groups on
- * equality and inequality. This is possible if all list items
are not at the
- * same level of a single BoolExpr expression, otherwise all of
them cannot be converted.
- */
-
+ /* At first, transform the arg and evaluate constant
expressions. */
orqual = transformExprRecurse(pstate, (Node *) arg);
orqual = coerce_to_boolean(pstate, orqual, "OR");
orqual = eval_const_expressions(NULL, orqual);
@@ -151,7 +139,13 @@ transformBoolExprOr(ParseState *pstate, BoolExpr
*expr_orig)
continue;
}
- /* Detect constant side of the clause */
+ /*
+ * Detect the constant side of the clause. Recall non-constant
+ * expression can be made not only with Vars, but also with
Params,
+ * which is not bonded with any relation. Thus, we detect the
const
+ * side - if another side is constant too, the orqual couldn't
be
+ * an OpExpr.
+ */
if (IsA(get_leftop(orqual), Const))
const_is_left = true;
else if (IsA(get_rightop(orqual), Const))
@@ -175,26 +169,14 @@ transformBoolExprOr(ParseState *pstate, BoolExpr
*expr_orig)
}
/*
- * The next step: transform all the inputs, and detect whether
- * any contain Vars.
- */
- if (!contain_vars_of_level(orqual, 0))
- {
- /* Again, it's not the expr we can transform */
- or_list = lappend(or_list, orqual);
- continue;
- }
-
- ;
-
- /*
- * At this point we definitely have a transformable
clause.
- * Classify it and add into specific group of clauses,
or create new
- * group.
- * TODO: to manage complexity in the case of many
different clauses
- * (X1=C1) OR (X2=C2 OR) ... (XN = CN) we could invent
something
- * like a hash table (htab key ???).
- */
+ * At this point we definitely have a transformable clause.
+ * Classify it and add into specific group of clauses, or create
new
+ * group.
+ * TODO: to manage complexity in the case of many different
clauses
+ * (X1=C1) OR (X2=C2 OR) ... (XN = CN) we could invent something
+ * like a hash table. But also we believe, that the case of many
+ * different variable sides is very rare.
+ */
foreach(lc_groups, groups_list)
{
OrClauseGroupEntry *v = (OrClauseGroupEntry *)
lfirst(lc_groups);
@@ -220,20 +202,18 @@ transformBoolExprOr(ParseState *pstate, BoolExpr
*expr_orig)
gentry = palloc(sizeof(OrClauseGroupEntry));
gentry->node = nconst_expr;
gentry->consts = list_make1(const_expr);
- gentry->opno = ((OpExpr *)orqual)->opno;
- gentry->expr = (Expr *)orqual;
+ gentry->expr = (Expr *) orqual;
groups_list = lappend(groups_list, (void *) gentry);
}
if (groups_list == NIL)
{
/*
- * No any transformations possible with this rinfo, just add
itself
- * to the list and go further.
+ * No any transformations possible with this list of arguments.
Here we
+ * already made all underlying transformations. Thus, just
return the
+ * transformed bool expression.
*/
- list_free(or_list);
-
- return transformBoolExpr(pstate, (BoolExpr *)expr_orig);
+ return (Node *) makeBoolExpr(OR_EXPR, or_list,
expr_orig->location);
}
else
{
@@ -267,11 +247,12 @@ transformBoolExprOr(ParseState *pstate, BoolExpr
*expr_orig)
}
/*
- * Do the transformation. It's been a long way ;)
+ * Do the transformation.
*
- * First of all, try to select a common type for the
array elements. Note that
- * since the LHS' type is first in the list, it will be
preferred when
- * there is doubt (eg, when all the RHS items are
unknown literals).
+ * First of all, try to select a common type for the
array elements.
+ * Note that since the LHS' type is first in the list,
it will be
+ * preferred when there is doubt (eg, when all the RHS
items are
+ * unknown literals).
*
* Note: use list_concat here not lcons, to avoid
damaging rnonvars.
*
@@ -288,8 +269,8 @@ transformBoolExprOr(ParseState *pstate, BoolExpr *expr_orig)
if (array_type != InvalidOid)
{
/*
- * OK: coerce all the right-hand non-Var inputs
to the common type
- * and build an ArrayExpr for them.
+ * OK: coerce all the right-hand non-Var inputs
to the common
+ * type and build an ArrayExpr for them.
*/
List *aexprs;
ArrayExpr *newa;
@@ -314,24 +295,18 @@ transformBoolExprOr(ParseState *pstate, BoolExpr
*expr_orig)
newa->array_typeid =
get_array_type(scalar_type);
newa->multidims = false;
newa->elements = aexprs;
- newa->location = -1; /* Position of the new
clause is undefined */
+ newa->location = -1;
- saopexpr = (ScalarArrayOpExpr
*)make_scalar_array_op(pstate,
-
list_make1(makeString((char *) "=")),
-
true,
-
gentry->node,
-
(Node *) newa,
-
-1); /* Position of the new clause is undefined */
-
- /*
- * TODO: here we can try to coerce the array to
a Const and find
- * hash func instead of linear search (see
50e17ad281b).
- * convert_saop_to_hashed_saop((Node *)
saopexpr);
- * We don't have to do this anymore, do we?
- */
+ saopexpr =
+ (ScalarArrayOpExpr *)
+ make_scalar_array_op(pstate,
+
list_make1(makeString((char *) "=")),
+
true,
+
gentry->node,
+
(Node *) newa,
+
-1);
or_list = lappend(or_list, (void *) saopexpr);
- change_apply = true;
}
else
{
@@ -344,23 +319,10 @@ transformBoolExprOr(ParseState *pstate, BoolExpr
*expr_orig)
list_free_deep(groups_list);
}
- if (change_apply)
- {
- /* One more trick: assemble correct clause */
- expr = list_length(or_list) > 1 ? makeBoolExpr(OR_EXPR,
list_copy(or_list), -1) : linitial(or_list);
- result = (Node *)expr;
- }
- /*
- * There was no reasons to create a new expresion, so
- * run the original BoolExpr conversion with using
- * transformBoolExpr function
- */
- else
- result = transformBoolExpr(pstate, (BoolExpr *)expr_orig);
-
- list_free(or_list);
-
- return result;
+ /* One more trick: assemble correct clause */
+ return (Node *) ((list_length(or_list) > 1) ?
+ makeBoolExpr(OR_EXPR, or_list,
expr_orig->location) :
+ linitial(or_list));
}
/*
@@ -475,7 +437,7 @@ transformExprRecurse(ParseState *pstate, Node *expr)
}
case T_BoolExpr:
- result = (Node *)transformBoolExprOr(pstate, (BoolExpr
*)expr);
+ result = (Node *)transformBoolExprOr(pstate, (BoolExpr
*) expr);
break;
case T_FuncCall:
