public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tom Lane <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: Richard Guo <[email protected]>
Cc: David Rowley <[email protected]>
Cc: jian he <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.
Date: Tue, 07 May 2024 14:20:55 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CACJufxFVQmr4=JWHAOSLuKA5Zy9H26nY6tVrRFBOekHoALyCkQ@mail.gmail.com>
	<CAApHDvpBMSGhXVeFdw_qmjPqU5yHvJJ7k0z7kD069EC4di2Pag@mail.gmail.com>
	<CAMbWs4-iYKxrMs8kj+49gVA4X1_=PYAaw5HL_3g_yVXpAHzYww@mail.gmail.com>
	<[email protected]>

Andrew Dunstan <[email protected]> writes:
> On 2024-05-07 Tu 06:05, Richard Guo wrote:
>> +1 to have build farm coverage of REALLOCATE_BITMAPSETS. This flag
>> seems quite useful.

> I have added it to the CPPFLAGS on prion.

... and as expected, prion fell over.

I find that Richard's proposed fix makes the core regression tests
pass, but we still fail check-world.  So I'm afraid we need something
more aggressive, like the attached which makes make_restrictinfo
copy all its input bitmapsets.  Without that, we still have sharing
of bitmapsets across different RestrictInfos, which seems pretty
scary given what we now see about the effects of 00b41463c.  This
seems annoyingly expensive, but maybe there's little choice?

Given this, we could remove ad-hoc bms_copy calls from the callers
of make_restrictinfo, distribute_quals_to_rels, etc.  I didn't go
looking for possible wins of that sort; there's unlikely to be a
lot of them.

			regards, tom lane



Attachments:

  [text/x-diff] copy-bitmapsets-when-making-restrictinfos.patch (1.1K, ../[email protected]/2-copy-bitmapsets-when-making-restrictinfos.patch)
  download | inline diff:
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 0b406e9334..e81861bc8b 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -132,8 +132,8 @@ make_restrictinfo_internal(PlannerInfo *root,
 	restrictinfo->is_clone = is_clone;
 	restrictinfo->can_join = false; /* may get set below */
 	restrictinfo->security_level = security_level;
-	restrictinfo->incompatible_relids = incompatible_relids;
-	restrictinfo->outer_relids = outer_relids;
+	restrictinfo->incompatible_relids = bms_copy(incompatible_relids);
+	restrictinfo->outer_relids = bms_copy(outer_relids);
 
 	/*
 	 * If it's potentially delayable by lower-level security quals, figure out
@@ -191,7 +191,7 @@ make_restrictinfo_internal(PlannerInfo *root,
 
 	/* required_relids defaults to clause_relids */
 	if (required_relids != NULL)
-		restrictinfo->required_relids = required_relids;
+		restrictinfo->required_relids = bms_copy(required_relids);
 	else
 		restrictinfo->required_relids = restrictinfo->clause_relids;
 


view thread (5+ 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]
  Subject: Re: Revert: Remove useless self-joins *and* -DREALLOCATE_BITMAPSETS make server crash, regress test fail.
  In-Reply-To: <[email protected]>

* 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