Hi!
On 20.02.2024 07:41, Andrei Lepikhov wrote:
On 20/2/2024 04:51, Tom Lane wrote:
Tomas Vondra <tomas.von...@enterprisedb.com> writes:
On 2/19/24 16:45, Tom Lane wrote:
Tomas Vondra <tomas.von...@enterprisedb.com> writes:
For example, I don't think we expect selectivity functions to
allocate
long-lived objects, right? So maybe we could run them in a dedicated
memory context, and reset it aggressively (after each call).
Here's a quick and probably-incomplete implementation of that idea.
I've not tried to study its effects on memory consumption, just made
sure it passes check-world.
Thanks for the sketch. The trick with the planner_tmp_cxt_depth
especially looks interesting.
I think we should design small memory contexts - one per scalable
direction of memory utilization, like selectivity or partitioning
(appending ?).
My coding experience shows that short-lived GEQO memory context forces
people to learn on Postgres internals more precisely :).
I think there was a problem in your patch when you freed up the memory
of a variable in the eqsel_internal function, because we have a case
where the variable was deleted by reference in the
eval_const_expressions_mutator function (it is only for T_SubPlan and
T_AlternativeSubPlan type of nodes.
This query just causes an error in your case:
create table a (id bigint, c1 bigint, primary key(id));
create table b (a_id bigint, b_id bigint, b2 bigint, primary key(a_id,
b_id));
explain select id
from a, b
where id = a_id
and b2 = (select min(b2)
from b
where id = a_id);
drop table a;
drop table b;
We can return a copy of the variable or not release the memory of this
variable.
I attached two patch: the first one is removing your memory cleanup and
another one returns the copy of variable.
The author of the corrections is not only me, but also Daniil Anisimov.
--
Regards,
Alena Rybakina
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/optimizer/util/clauses.c
b/src/backend/optimizer/util/clauses.c
index edc25d712e9..ac560b1605e 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -2915,7 +2915,7 @@ eval_const_expressions_mutator(Node *node,
* XXX should we ereport() here instead? Probably this
routine
* should never be invoked after SubPlan creation.
*/
- return node;
+ return CopyObject(node);
case T_RelabelType:
{
RelabelType *relabel = (RelabelType *) node;
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index cea777e9d40..e5bad75ec1c 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -281,6 +281,7 @@ eqsel_internal(PG_FUNCTION_ARGS, bool negate)
selec = var_eq_non_const(&vardata, operator, collation, other,
varonleft,
negate);
+ pfree(other);
ReleaseVariableStats(vardata);
return selec;
@@ -1961,15 +1962,15 @@ scalararraysel(PlannerInfo *root,
{
List *args;
Selectivity s2;
-
- args = list_make2(leftop,
-
makeConst(nominal_element_type,
-
-1,
-
nominal_element_collation,
-
elmlen,
-
elem_values[i],
-
elem_nulls[i],
-
elmbyval));
+ Const *c = makeConst(nominal_element_type,
+ -1,
+
nominal_element_collation,
+ elmlen,
+ elem_values[i],
+ elem_nulls[i],
+ elmbyval);
+
+ args = list_make2(leftop, c);
if (is_join_clause)
s2 =
DatumGetFloat8(FunctionCall5Coll(&oprselproc,
clause->inputcollid,
@@ -1985,7 +1986,8 @@ scalararraysel(PlannerInfo *root,
ObjectIdGetDatum(operator),
PointerGetDatum(args),
Int32GetDatum(varRelid)));
-
+ list_free(args);
+ pfree(c);
if (useOr)
{
s1 = s1 + s2 - s1 * s2;
@@ -2004,6 +2006,9 @@ scalararraysel(PlannerInfo *root,
if ((useOr ? isEquality : isInequality) &&
s1disjoint >= 0.0 && s1disjoint <= 1.0)
s1 = s1disjoint;
+
+ pfree(elem_values);
+ pfree(elem_nulls);
}
else if (rightop && IsA(rightop, ArrayExpr) &&
!((ArrayExpr *) rightop)->multidims)
@@ -2052,6 +2057,7 @@ scalararraysel(PlannerInfo *root,
ObjectIdGetDatum(operator),
PointerGetDatum(args),
Int32GetDatum(varRelid)));
+ list_free(args);
if (useOr)
{
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index cea777e9d40..9477ce0706b 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -1961,15 +1961,15 @@ scalararraysel(PlannerInfo *root,
{
List *args;
Selectivity s2;
-
- args = list_make2(leftop,
-
makeConst(nominal_element_type,
-
-1,
-
nominal_element_collation,
-
elmlen,
-
elem_values[i],
-
elem_nulls[i],
-
elmbyval));
+ Const *c = makeConst(nominal_element_type,
+ -1,
+
nominal_element_collation,
+ elmlen,
+ elem_values[i],
+ elem_nulls[i],
+ elmbyval);
+
+ args = list_make2(leftop, c);
if (is_join_clause)
s2 =
DatumGetFloat8(FunctionCall5Coll(&oprselproc,
clause->inputcollid,
@@ -1985,7 +1985,8 @@ scalararraysel(PlannerInfo *root,
ObjectIdGetDatum(operator),
PointerGetDatum(args),
Int32GetDatum(varRelid)));
-
+ list_free(args);
+ pfree(c);
if (useOr)
{
s1 = s1 + s2 - s1 * s2;
@@ -2004,6 +2005,9 @@ scalararraysel(PlannerInfo *root,
if ((useOr ? isEquality : isInequality) &&
s1disjoint >= 0.0 && s1disjoint <= 1.0)
s1 = s1disjoint;
+
+ pfree(elem_values);
+ pfree(elem_nulls);
}
else if (rightop && IsA(rightop, ArrayExpr) &&
!((ArrayExpr *) rightop)->multidims)
@@ -2052,6 +2056,7 @@ scalararraysel(PlannerInfo *root,
ObjectIdGetDatum(operator),
PointerGetDatum(args),
Int32GetDatum(varRelid)));
+ list_free(args);
if (useOr)
{
