Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)

public inbox for [email protected]  
help / color / mirror / Atom feed

From: David Rowley <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: Chao Li <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Xuneng Zhou <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Date: Fri, 27 Mar 2026 12:10:15 +1300
Message-ID: <CAApHDvqAOeOwCKh9g0gfxWa040=Hyc7_oA=C59rjod8kXJDWyw@mail.gmail.com> (raw)
In-Reply-To: <CAAKRu_ZrDadxmGepBwPZ03yAKnMxwsHYn8SK9Gg7VqigLLVUWg@mail.gmail.com>
References: <CAAKRu_Z8Ry_ynNBPAzs_Ry3MQi9NaBgt1ccLgwRsDbxWpocaBg@mail.gmail.com>
	<CAAKRu_ZbOp52rnkjf63h5mf94raEKBH7AAbz6QTx-xdH9yLfmQ@mail.gmail.com>
	<CAAKRu_b8m+iuupm4ZX+2_V5Xj5u4jCTrU=Tv=epg6p4H2SMkFQ@mail.gmail.com>
	<CALdSSPh9hVXNiPwdntWqbMzu5upKy0jBDDe7Un0_Nf2A54R2VQ@mail.gmail.com>
	<CAAKRu_a6Cd7JnxhY4A=b_Paxc3UDUDOPeqV3GbzMh=R2KkD-uQ@mail.gmail.com>
	<jlsvov4o5xswxjvjhvuchz6y55ncvoc457grvxct7cub5gcxuj@4e2toesujnpr>
	<CAAKRu_bsHbvt+VqbjHXsdphKf8fqwBEutRhH3fmo+qUVe=yBKw@mail.gmail.com>
	<CAAKRu_ZhHtEaucO--SdYrCjq0zgqk_LPztUD+-QS74A2htXgKw@mail.gmail.com>
	<CAAKRu_Zj8G4T=HN3QSY7iQvkKSQk-k1fq+eJkjCBNqoSg63z+Q@mail.gmail.com>
	<CAAKRu_bgP-DMZs=D2j2N0+U9+uWU5cVagw-yZLOuhYbWj_KwnA@mail.gmail.com>
	<itvgqc6vncbjsjfmrptfvkkeg5vqzhalaguya2z77t6c6ctpc3@wsdrgbn4bxaa>
	<CAAKRu_aWMyGB=zg5W7+RUtor6TqsiOwHXSL7Dg4TUUiTSzzcpw@mail.gmail.com>
	<[email protected]>
	<CAAKRu_Ypa7-JGVR+fstDxU5Cfitk_rf5ijdaqwtoPkztursufA@mail.gmail.com>
	<CAAKRu_ZrDadxmGepBwPZ03yAKnMxwsHYn8SK9Gg7VqigLLVUWg@mail.gmail.com>

On Thu, 26 Mar 2026 at 12:14, Melanie Plageman
<[email protected]> wrote:
> Attached v46 addresses your feedback and has a bit of assorted cleanup in it.

(I've not had a chance to process this thread, so apologies if I
missed discussion on certain things I'm going to say)

I was looking at v46-0001. With:

+++ b/src/include/nodes/plannodes.h
@@ -112,6 +112,12 @@ typedef struct PlannedStmt
  */
  Bitmapset  *unprunableRelids;

+ /*
+ * RT indexes of relations modified by the query through
+ * UPDATE/DELETE/INSERT/MERGE or targeted by SELECT FOR UPDATE/SHARE.
+ */
+ Bitmapset  *modifiedRelids;
+

This doesn't really mention anything about leaf partitions not being
mentioned for INSERT queries. You did mention it in standard_planner()
here:

+ * modification. Conversely, leaf partitions whose result relations are
+ * created at the time of insert are not included here.

I think if someone is going to use this field, they're going to look
at where the field is defined to find out what it is, not where it
gets populated.

I'm also wondering about having this combined field. If you were to
have a Bitmapset field that mirrors "List *resultRelations;", then
have another:

/* a list of PlanRowMark's */
List   *rowMarks;

+ /* Relids which have rowMarks */
+ Bitmapset *rowMarkRelids;

I think they're more likely to be useful for other purposes, and I
think the only pain that it causes you is that you have to call
bms_is_member() twice in ScanRelIsReadOnly().

Then, as a follow-up, maybe we could consider removing
PlannedStmt.resultRelations.  (The deprecated)
ExecRelationIsTargetRelation() could use the new Bitmapset, which
would be more efficient. OverExplain does do:

if (es->format != EXPLAIN_FORMAT_TEXT ||
plannedstmt->resultRelations != NIL)
overexplain_intlist("Result RTIs", plannedstmt->resultRelations, es);

but maybe Robert is ok with those coming out in ascending numerical
order rather than list order. overexplain_bitmapset() would do that.

In [1], I didn't see any code actually using the field. Just a couple
of projects that have duplicated the copyObject() code.

I did quickly look over the remaining patches. I wondered if you might
want to add a new ScanOption SO_NONE = 0, or SO_EMPTY_FLAGS. It might
make the places where you're passing zero directly easier to read?

David

[1] https://codesearch.debian.net/search?q=resultRelations&literal=1

view thread (144+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
  In-Reply-To: <CAApHDvqAOeOwCKh9g0gfxWa040=Hyc7_oA=C59rjod8kXJDWyw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox