MIME-Version: 1.0
References: <ddc34dbd-1efc-4710-824c-e101e7eb63e6@postgrespro.ru>
In-Reply-To: <ddc34dbd-1efc-4710-824c-e101e7eb63e6@postgrespro.ru>
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Mon, 30 Mar 2026 17:26:35 +0200
Message-ID: 
 <CAEze2Wh+_C8LtmiMRb58df=A1PrBVmMnYMOfbBUk9c=m99CN+w@mail.gmail.com>
Subject: Re: [BUG] Excessive memory usage with update on STORED generated
 columns.
To: "Anton A. Melnikov" <a.melnikov@postgrespro.ru>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000a7503e064e3f7ae6"
Archived-At: 
 <https://www.postgresql.org/message-id/CAEze2Wh%2B_C8LtmiMRb58df%3DA1PrBVmMnYMOfbBUk9c%3Dm99CN%2Bw%40mail.gmail.com>
Precedence: bulk

--000000000000a7503e064e3f7ae6
Content-Type: text/plain; charset="UTF-8"

On Mon, 30 Mar 2026 at 16:25, Anton A. Melnikov
<a.melnikov@postgrespro.ru> wrote:
>
> Hi!
>
>
> My colleagues found that a queries like that:
[...]
> lead to excessive memory consumption up to 10Gb in this example and
> query execution time up to ~1,5min.
>
> Bisect shows that the problem appeared after commit 83ea6c540
> (Virtual generated columns).

Yep, that looks about accurate. Thanks for the report and initial triage!

> This indicates that generated expressions are reparsed multiple times,
> once per row to be updated instead of being reused.
[...]
> I would like to propose a fix that add a caching of the the parsed
> expression trees (Node *) in ResultRelInfo, so that build_column_default()
> and stringToNode() are executed at most once per attribute per query.
>
> With this fix, the query execution time
> and memory consumption return to normal:

I think that fixes the wrong issue. Specifically, ExecInitGenerated
mentions that it expects to be called just once per ResultRelInfo, and
your code adds code to allow it to be called more than once without
causing more problems. While it does solve the complaint, it's still
executing more work than it should. Additionally, though it's not a
failing of the patch per se, it changes the layout of an existing
struct, and so it isn't backportable.

The actual issue is that ExecComputeStoredGenerated uses
ri_GeneratedExprsU's NULL-ness to check whether the generated columns'
expressions have been initialized, whilst for UPDATE ResultRelInfos
the initialized-indicator is stored in ri_extraUpdatedCols_valid.

So, I think the attached patch is a more appropriate fix, it avoids
calling into ExecInitGenerated at all when no column included in
expressions was updated. It also adds an assertion that the function
isn't called again once the field has been initialized. It also has
the benefit of being backportable.


Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

--000000000000a7503e064e3f7ae6
Content-Type: application/octet-stream;
	name="v2-0001-nodeModifyTable-fix-generated-tables-memory-leak-.patch"
Content-Disposition: attachment;
	filename="v2-0001-nodeModifyTable-fix-generated-tables-memory-leak-.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mndc42om0>
X-Attachment-Id: f_mndc42om0

RnJvbSAxOGVmZGRlMTBlYmJhYWIwOWU4ZDE2NmUyNzFkNGRhYzAxNDYxYTY4IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXR0aGlhcyB2YW4gZGUgTWVlbnQgPGJvZWtld3VybStwb3N0
Z3Jlc0BnbWFpbC5jb20+CkRhdGU6IE1vbiwgMzAgTWFyIDIwMjYgMTc6MTU6MjQgKzAyMDAKU3Vi
amVjdDogW1BBVENIIHYyXSBub2RlTW9kaWZ5VGFibGU6IGZpeCBnZW5lcmF0ZWQgdGFibGVzIG1l
bW9yeSBsZWFrL3BlcmYKIGlzc3VlCgpUaGUgY29kZSBhc3N1bWVkIHRoYXQgcmlfR2VuZXJhdGVk
RXhwcnNVIHdvdWxkIGFsd2F5cyBjb250YWluIGEgbGlzdCBvZiBleHByZXNzaW9ucyBpZiB0aGUg
Z2VuZXJhdGVkIGNvbHVtcyB3ZXJlIGNoZWNrZWQsIGJ1dCB0aGF0J3Mgbm90IGNvcnJlY3QuCgpS
ZXBvcnRlZC1ieTogIkFudG9uIEEuIE1lbG5pa292IiA8YS5tZWxuaWtvdkBwb3N0Z3Jlc3Byby5y
dT4KLS0tCiBzcmMvYmFja2VuZC9leGVjdXRvci9ub2RlTW9kaWZ5VGFibGUuYyB8IDMgKystCiAx
IGZpbGUgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCgpkaWZmIC0tZ2l0
IGEvc3JjL2JhY2tlbmQvZXhlY3V0b3Ivbm9kZU1vZGlmeVRhYmxlLmMgYi9zcmMvYmFja2VuZC9l
eGVjdXRvci9ub2RlTW9kaWZ5VGFibGUuYwppbmRleCA0Y2Q1ZTI2MmUwZi4uMDlkZDUwMmZiOGUg
MTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL2V4ZWN1dG9yL25vZGVNb2RpZnlUYWJsZS5jCisrKyBi
L3NyYy9iYWNrZW5kL2V4ZWN1dG9yL25vZGVNb2RpZnlUYWJsZS5jCkBAIC01MzEsNiArNTMxLDcg
QEAgRXhlY0luaXRHZW5lcmF0ZWQoUmVzdWx0UmVsSW5mbyAqcmVzdWx0UmVsSW5mbywKIAl7CiAJ
CS8qIERvbid0IGNhbGwgdHdpY2UgKi8KIAkJQXNzZXJ0KHJlc3VsdFJlbEluZm8tPnJpX0dlbmVy
YXRlZEV4cHJzVSA9PSBOVUxMKTsKKwkJQXNzZXJ0KCFyZXN1bHRSZWxJbmZvLT5yaV9leHRyYVVw
ZGF0ZWRDb2xzX3ZhbGlkKTsKIAogCQlyZXN1bHRSZWxJbmZvLT5yaV9HZW5lcmF0ZWRFeHByc1Ug
PSByaV9HZW5lcmF0ZWRFeHByczsKIAkJcmVzdWx0UmVsSW5mby0+cmlfTnVtR2VuZXJhdGVkTmVl
ZGVkVSA9IHJpX051bUdlbmVyYXRlZE5lZWRlZDsKQEAgLTU3NSw3ICs1NzYsNyBAQCBFeGVjQ29t
cHV0ZVN0b3JlZEdlbmVyYXRlZChSZXN1bHRSZWxJbmZvICpyZXN1bHRSZWxJbmZvLAogCSAqLwog
CWlmIChjbWR0eXBlID09IENNRF9VUERBVEUpCiAJewotCQlpZiAocmVzdWx0UmVsSW5mby0+cmlf
R2VuZXJhdGVkRXhwcnNVID09IE5VTEwpCisJCWlmICghcmVzdWx0UmVsSW5mby0+cmlfZXh0cmFV
cGRhdGVkQ29sc192YWxpZCkKIAkJCUV4ZWNJbml0R2VuZXJhdGVkKHJlc3VsdFJlbEluZm8sIGVz
dGF0ZSwgY21kdHlwZSk7CiAJCWlmIChyZXN1bHRSZWxJbmZvLT5yaV9OdW1HZW5lcmF0ZWROZWVk
ZWRVID09IDApCiAJCQlyZXR1cm47Ci0tIAoyLjUwLjEgKEFwcGxlIEdpdC0xNTUpCgo=
--000000000000a7503e064e3f7ae6--