Hi Hackers,
I received an off-list memory leak report from Tomas Vondra. Details below:
> CREATE DOMAIN leakdom AS daterange CHECK (VALUE IS NULL OR NOT
> isempty(VALUE));
>
> CREATE TABLE leak (id int, valid_at leakdom, name text);
> --CREATE TABLE leak (id int, valid_at daterange, name text);
>
> INSERT INTO leak
> SELECT g, daterange('2000-01-01','2010-01-01'), 'x'
> FROM generate_series(1, 200_000) g;
>
> SET log_executor_stats = on;
>
> UPDATE leak FOR PORTION OF valid_at FROM '2004-01-01' TO '2005-01-01'
> SET name = 'y';
> ---
>
> With the domain, I get this:
>
> DETAIL: ! system usage stats:
> ! 6.003382 s user, 1.508554 s system, 7.633776 s elapsed
> ! [6.724448 s user, 1.533475 s system total]
> ! 2633248 kB max resident size
> ! 0/74240 [0/114192] filesystem blocks in/out
> ! 0/646011 [0/651587] page faults/reclaims, 0 [0] swaps
> ! 0 [0] signals rcvd, 0/0 [0/0] messages rcvd/sent
> ! 13/309 [31/354] voluntary/involuntary context switches
>
> while with the plain daterange I get
>
> DETAIL: ! system usage stats:
> ! 3.857666 s user, 0.071827 s system, 4.046457 s elapsed
> ! [4.517879 s user, 0.114015 s system total]
> ! 79060 kB max resident size
> ! 0/92096 [0/149952] filesystem blocks in/out
> ! 0/7674 [0/14166] page faults/reclaims, 0 [0] swaps
> ! 0 [0] signals rcvd, 0/0 [0/0] messages rcvd/sent
> ! 23/130 [39/159] voluntary/involuntary context switches
>
> That's ~2.6GB vs. ~80MB for the RSS, which seems like a lot. It can be
> made worse by using more rows in the table.
>
> AFAIK the issues is in nodeModifyTable.c, which does this:
>
> /*
> * Does the new Datum violate domain checks? Row-level CHECK
> * constraints are validated by ExecInsert, so we don't need to do
> * anything here for those.
> */
> if (forPortionOf->isDomain)
> domain_check(leftover, false, forPortionOf->rangeVar->vartype,
> NULL, NULL);
>
> where the NULLs mean it's running with CurrentMemoryContext, which is
> es_query_ctx. And hence the query-wide leak.
>
> domain_check() has a way to cache stuff once - that's what the extra
> argument is about. I don't know enough about this code, maybe it could
> be a local scratch space, but maybe it'd be better to add it to the FPO
> executor state:
>
> /* src/include/nodes/execnodes.h — ForPortionOfState */
> void *fp_domaininfo; /* cache space for domain_check() */
>
> and call it like this:
>
> if (forPortionOf->isDomain)
> domain_check(leftover, false, forPortionOf->rangeVar->vartype,
> &fpoState->fp_domaininfo, NULL);
I agree that is clearly a leak. His fix works and makes the memory
usage consistent between the domain and non-domain cases. My only
change to the above is to pass estate->es_query_cxt explicitly instead
of assuming it's already the CurrentMemoryContext. I also added a test
with a multi-row domain FOR PORTION OF update to validate that the
caching functions correctly.
Yours,
--
Paul ~{:-)
[email protected]
From 99e277b8afe18a8e8c850fb9c8ba53b418ef6297 Mon Sep 17 00:00:00 2001
From: "Paul A. Jungwirth" <[email protected]>
Date: Fri, 4 Sep 2026 15:25:03 -0700
Subject: [PATCH v1] Fix memory leak in FOR PORTION OF domain lookup
ExecForPortionOfLeftovers() calls domain_check() on every row (actually every
temporal leftover) with NULL for the "extra" parameter, which can be used to
cache the DomainIOData. Without a cache, all the DomainIODatas accumulate and
never get freed.
This commit adds a field to ForPortionOfState to hold the DomainIOData. We pass
that and the query memory context to domain_check, so that we now build the
domain data once.
The test here doesn't exercise the memory leak, but it makes sure that a
multi-row UPDATE FOR PORTION OF with domains isn't broken by the caching.
Reported-by: Tomas Vondra <[email protected]>
Author: Paul A. Jungwirth <[email protected]>
Backpatch-through: 19
---
src/backend/executor/nodeModifyTable.c | 3 +-
src/include/nodes/execnodes.h | 1 +
src/test/regress/expected/for_portion_of.out | 50 ++++++++++++++++++++
src/test/regress/sql/for_portion_of.sql | 26 ++++++++++
4 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 5681505d31c..f12e30573be 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1554,7 +1554,8 @@ ExecForPortionOfLeftovers(ModifyTableContext *context,
* anything here for those.
*/
if (forPortionOf->isDomain)
- domain_check(leftover, false, forPortionOf->rangeVar->vartype, NULL, NULL);
+ domain_check(leftover, false, forPortionOf->rangeVar->vartype,
+ &fpoState->fp_domaininfo, estate->es_query_cxt);
if (!didInit)
{
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e95ac3eda35..1efb1f871b5 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -483,6 +483,7 @@ typedef struct ForPortionOfState
TypeCacheEntry *fp_leftoverstypcache; /* type cache entry of the range */
TupleTableSlot *fp_Existing; /* slot to store old tuple */
TupleTableSlot *fp_Leftover; /* slot to store leftover */
+ void *fp_domaininfo; /* cache space for domain_check() */
} ForPortionOfState;
/*
diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out
index 64789d1777b..8937f84c157 100644
--- a/src/test/regress/expected/for_portion_of.out
+++ b/src/test/regress/expected/for_portion_of.out
@@ -1176,6 +1176,56 @@ SELECT * FROM for_portion_of_test2 WHERE id = 2 ORDER BY valid_at;
2 | [2010-01-09,2020-01-01) | two
(3 rows)
+DROP TABLE for_portion_of_test2;
+-- The domain is checked for every leftover, not just the first one. The
+-- lookup is cached across rows, so make sure a violation is still caught on a
+-- row after one that passed.
+CREATE TABLE for_portion_of_test2 (
+ id integer,
+ valid_at daterange_d,
+ name text
+);
+INSERT INTO for_portion_of_test2 VALUES
+ (1, '[2006-01-01,2020-01-01)', 'one'),
+ (2, '[2000-01-01,2020-01-01)', 'two'),
+ (3, '[2000-01-01,2020-01-01)', 'three');
+-- Every row's leftovers are fine here: several rows, several checks.
+UPDATE for_portion_of_test2
+ FOR PORTION OF valid_at FROM '2010-01-01' TO '2011-01-01'
+ SET name = name || '!';
+SELECT * FROM for_portion_of_test2 ORDER BY id, valid_at;
+ id | valid_at | name
+----+-------------------------+--------
+ 1 | [2006-01-01,2010-01-01) | one
+ 1 | [2010-01-01,2011-01-01) | one!
+ 1 | [2011-01-01,2020-01-01) | one
+ 2 | [2000-01-01,2010-01-01) | two
+ 2 | [2010-01-01,2011-01-01) | two!
+ 2 | [2011-01-01,2020-01-01) | two
+ 3 | [2000-01-01,2010-01-01) | three
+ 3 | [2010-01-01,2011-01-01) | three!
+ 3 | [2011-01-01,2020-01-01) | three
+(9 rows)
+
+-- Now the first row's leftovers pass but the second row's violate the domain.
+UPDATE for_portion_of_test2
+ FOR PORTION OF valid_at FROM '2005-05-05' TO '2007-01-01'
+ SET name = 'nope';
+ERROR: value for domain daterange_d violates check constraint "daterange_d_check"
+SELECT * FROM for_portion_of_test2 ORDER BY id, valid_at;
+ id | valid_at | name
+----+-------------------------+--------
+ 1 | [2006-01-01,2010-01-01) | one
+ 1 | [2010-01-01,2011-01-01) | one!
+ 1 | [2011-01-01,2020-01-01) | one
+ 2 | [2000-01-01,2010-01-01) | two
+ 2 | [2010-01-01,2011-01-01) | two!
+ 2 | [2011-01-01,2020-01-01) | two
+ 3 | [2000-01-01,2010-01-01) | three
+ 3 | [2010-01-01,2011-01-01) | three!
+ 3 | [2011-01-01,2020-01-01) | three
+(9 rows)
+
DROP TABLE for_portion_of_test2;
-- With a domain on a multirangetype
CREATE FUNCTION multirange_lowers(mr anymultirange) RETURNS anyarray LANGUAGE sql AS $$
diff --git a/src/test/regress/sql/for_portion_of.sql b/src/test/regress/sql/for_portion_of.sql
index b61fe10478e..315921cf872 100644
--- a/src/test/regress/sql/for_portion_of.sql
+++ b/src/test/regress/sql/for_portion_of.sql
@@ -764,6 +764,32 @@ ALTER TABLE for_portion_of_test2 DROP CONSTRAINT fpo2_check;
SELECT * FROM for_portion_of_test2 WHERE id = 2 ORDER BY valid_at;
DROP TABLE for_portion_of_test2;
+-- The domain is checked for every leftover, not just the first one. The
+-- lookup is cached across rows, so make sure a violation is still caught on a
+-- row after one that passed.
+CREATE TABLE for_portion_of_test2 (
+ id integer,
+ valid_at daterange_d,
+ name text
+);
+INSERT INTO for_portion_of_test2 VALUES
+ (1, '[2006-01-01,2020-01-01)', 'one'),
+ (2, '[2000-01-01,2020-01-01)', 'two'),
+ (3, '[2000-01-01,2020-01-01)', 'three');
+
+-- Every row's leftovers are fine here: several rows, several checks.
+UPDATE for_portion_of_test2
+ FOR PORTION OF valid_at FROM '2010-01-01' TO '2011-01-01'
+ SET name = name || '!';
+SELECT * FROM for_portion_of_test2 ORDER BY id, valid_at;
+
+-- Now the first row's leftovers pass but the second row's violate the domain.
+UPDATE for_portion_of_test2
+ FOR PORTION OF valid_at FROM '2005-05-05' TO '2007-01-01'
+ SET name = 'nope';
+SELECT * FROM for_portion_of_test2 ORDER BY id, valid_at;
+DROP TABLE for_portion_of_test2;
+
-- With a domain on a multirangetype
CREATE FUNCTION multirange_lowers(mr anymultirange) RETURNS anyarray LANGUAGE sql AS $$
SELECT array_agg(lower(r)) FROM UNNEST(mr) u(r);
--
2.47.3
