public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Richard Guo <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Assert failure on 'list_member_ptr(rel->joininfo, restrictinfo)'
Date: Fri, 24 Nov 2023 15:54:27 +0200
Message-ID: <CAPpHfdtLgCryACcrmLv=Koq9rAB3=tr5y9D84dGgvUhSCvjzjg@mail.gmail.com> (raw)
In-Reply-To: <CAMbWs49oGdqF7aLg3OsSjo3tRFgU=vUAtKC28DtWFZNds5mc=Q@mail.gmail.com>
References: <CAMbWs4_wJthNtYBL+SsebpgF-5L2r5zFFk6xYbS0A78GKOTFHw@mail.gmail.com>
<[email protected]>
<CAPpHfduuEve8ezjqNEmCFuH3+fGZ2z+gzFV_xGD1ENSgqKTAZA@mail.gmail.com>
<CAPpHfdugmUqPhr40xmbk5xMQjy96FoJzC0fE6KYBoNbZYHVeDA@mail.gmail.com>
<CAMbWs49oGdqF7aLg3OsSjo3tRFgU=vUAtKC28DtWFZNds5mc=Q@mail.gmail.com>
On Thu, Nov 23, 2023 at 4:33 AM Richard Guo <[email protected]> wrote:
>
> On Sun, Nov 19, 2023 at 9:17 AM Alexander Korotkov <[email protected]> wrote:
>>
>> It's here. New REALLOCATE_BITMAPSETS forces bitmapset reallocation on
>> each modification.
>
>
> +1 to the idea of introducing a reallocation mode to Bitmapset.
>
>>
>> I had the feeling of falling into a rabbit hole while debugging all
>> the cases of failure with this new option. With the second patch
>> regressions tests pass.
>
>
> It seems to me that we have always had situations where we share the
> same pointer to a Bitmapset structure across different places. I do not
> think this is a problem as long as we do not modify the Bitmapsets in a
> way that requires reallocation or impact the locations sharing the same
> pointer.
>
> So I'm wondering, instead of attempting to avoid sharing pointer to
> Bitmapset in all locations that have problems, can we simply bms_copy
> the original Bitmapset within replace_relid() before making any
> modifications, as I proposed previously? Of course, as Andres pointed
> out, we need to do so also for the "Delete relid without substitution"
> path. Please see the attached.
Yes, this makes sense. Thank you for the patch. My initial point was
that replace_relid() should either do in-place in all cases or make a
copy in all cases. Now I see that it should make a copy in all cases.
Note, that without making a copy in delete case, regression tests fail
with REALLOCATE_BITMAPSETS on.
Please, find the revised patchset. As Ashutosh Bapat asked, asserts
are split into separate patch.
------
Regards,
Alexander Korotkov
Attachments:
[application/octet-stream] 0003-Make-replace_relid-leave-argument-unmodified-v2.patch (3.6K, ../CAPpHfdtLgCryACcrmLv=Koq9rAB3=tr5y9D84dGgvUhSCvjzjg@mail.gmail.com/2-0003-Make-replace_relid-leave-argument-unmodified-v2.patch)
download | inline diff:
From c197405fbae7f6b8abe217fbfab8c4882a872229 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 24 Nov 2023 14:57:27 +0200
Subject: [PATCH 3/3] Make replace_relid() leave argument unmodified
There are a lot of situations when we share the same pointer to a Bitmapset
structure across different places. In order to evade undesirable side effects
replace_relid() function should always return a copy.
---
src/backend/optimizer/plan/analyzejoins.c | 11 ++++++++---
src/test/regress/expected/join.out | 15 +++++++++++++++
src/test/regress/sql/join.sql | 6 ++++++
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index b9be19a687f..75e4ebc64a2 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1511,6 +1511,10 @@ replace_varno_walker(Node *node, ReplaceVarnoContext *ctx)
/*
* Substitute newId by oldId in relids.
+ *
+ * We must make a copy of the original Bitmapset before making any
+ * modifications, because the same pointer to it might be shared among
+ * different places.
*/
static Bitmapset *
replace_relid(Relids relids, int oldId, int newId)
@@ -1518,12 +1522,13 @@ replace_relid(Relids relids, int oldId, int newId)
if (oldId < 0)
return relids;
+ /* Delete relid without substitution. */
if (newId < 0)
- /* Delete relid without substitution. */
- return bms_del_member(relids, oldId);
+ return bms_del_member(bms_copy(relids), oldId);
+ /* Substitute newId for oldId. */
if (bms_is_member(oldId, relids))
- return bms_add_member(bms_del_member(relids, oldId), newId);
+ return bms_add_member(bms_del_member(bms_copy(relids), oldId), newId);
return relids;
}
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 2c73270143b..5ceecc44132 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -6836,6 +6836,21 @@ select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
-> Function Scan on generate_series t3
(6 rows)
+-- Check that SJE replaces join clauses involving the removed rel correctly
+explain (costs off)
+select * from emp1 t1
+ inner join emp1 t2 on t1.id = t2.id
+ left join emp1 t3 on t1.id > 1 and t1.id < 2;
+ QUERY PLAN
+----------------------------------------------
+ Nested Loop Left Join
+ Join Filter: ((t2.id > 1) AND (t2.id < 2))
+ -> Seq Scan on emp1 t2
+ Filter: (id IS NOT NULL)
+ -> Materialize
+ -> Seq Scan on emp1 t3
+(6 rows)
+
-- We can remove the join even if we find the join can't duplicate rows and
-- the base quals of each side are different. In the following case we end up
-- moving quals over to s1 to make it so it can't match any rows.
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 8a8a63bd2f1..c3ec340e2da 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2606,6 +2606,12 @@ explain (costs off)
select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
lateral (select t1.id as t1id, * from generate_series(1,1) t3) s on true;
+-- Check that SJE replaces join clauses involving the removed rel correctly
+explain (costs off)
+select * from emp1 t1
+ inner join emp1 t2 on t1.id = t2.id
+ left join emp1 t3 on t1.id > 1 and t1.id < 2;
+
-- We can remove the join even if we find the join can't duplicate rows and
-- the base quals of each side are different. In the following case we end up
-- moving quals over to s1 to make it so it can't match any rows.
--
2.39.3 (Apple Git-145)
[application/octet-stream] 0001-Add-asserts-to-bimapset-manipulation-functions-v2.patch (8.7K, ../CAPpHfdtLgCryACcrmLv=Koq9rAB3=tr5y9D84dGgvUhSCvjzjg@mail.gmail.com/3-0001-Add-asserts-to-bimapset-manipulation-functions-v2.patch)
download | inline diff:
From 33b7e3859797150d0d54bd08ae202663d9f754d0 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Fri, 24 Nov 2023 15:32:32 +0200
Subject: [PATCH 1/3] Add asserts to bimapset manipulation functions
New asserts validate that arguments are really bitmapsets. This should help
to early detect accesses to dangling pointers.
---
src/backend/nodes/bitmapset.c | 77 ++++++++++++++++++++++++++++-------
1 file changed, 63 insertions(+), 14 deletions(-)
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 704879f5660..1627922ef7a 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -84,6 +84,7 @@ bms_copy(const Bitmapset *a)
if (a == NULL)
return NULL;
+ Assert(IsA(a, Bitmapset));
size = BITMAPSET_SIZE(a->nwords);
result = (Bitmapset *) palloc(size);
memcpy(result, a, size);
@@ -98,8 +99,8 @@ bms_equal(const Bitmapset *a, const Bitmapset *b)
{
int i;
- Assert(a == NULL || a->words[a->nwords - 1] != 0);
- Assert(b == NULL || b->words[b->nwords - 1] != 0);
+ Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
+ Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
/* Handle cases where either input is NULL */
if (a == NULL)
@@ -139,8 +140,8 @@ bms_compare(const Bitmapset *a, const Bitmapset *b)
{
int i;
- Assert(a == NULL || a->words[a->nwords - 1] != 0);
- Assert(b == NULL || b->words[b->nwords - 1] != 0);
+ Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
+ Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
/* Handle cases where either input is NULL */
if (a == NULL)
@@ -215,6 +216,9 @@ bms_union(const Bitmapset *a, const Bitmapset *b)
int otherlen;
int i;
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
+
/* Handle cases where either input is NULL */
if (a == NULL)
return bms_copy(b);
@@ -253,6 +257,9 @@ bms_intersect(const Bitmapset *a, const Bitmapset *b)
int resultlen;
int i;
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
+
/* Handle cases where either input is NULL */
if (a == NULL || b == NULL)
return NULL;
@@ -299,8 +306,8 @@ bms_difference(const Bitmapset *a, const Bitmapset *b)
Bitmapset *result;
int i;
- Assert(a == NULL || a->words[a->nwords - 1] != 0);
- Assert(b == NULL || b->words[b->nwords - 1] != 0);
+ Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
+ Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
/* Handle cases where either input is NULL */
if (a == NULL)
@@ -308,6 +315,8 @@ bms_difference(const Bitmapset *a, const Bitmapset *b)
if (b == NULL)
return bms_copy(a);
+ Assert(IsA(a, Bitmapset) && IsA(b, Bitmapset));
+
/*
* In Postgres' usage, an empty result is a very common case, so it's
* worth optimizing for that by testing bms_nonempty_difference(). This
@@ -364,8 +373,8 @@ bms_is_subset(const Bitmapset *a, const Bitmapset *b)
{
int i;
- Assert(a == NULL || a->words[a->nwords - 1] != 0);
- Assert(b == NULL || b->words[b->nwords - 1] != 0);
+ Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
+ Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
/* Handle cases where either input is NULL */
if (a == NULL)
@@ -373,6 +382,8 @@ bms_is_subset(const Bitmapset *a, const Bitmapset *b)
if (b == NULL)
return false;
+ Assert(IsA(a, Bitmapset) && IsA(b, Bitmapset));
+
/* 'a' can't be a subset of 'b' if it contains more words */
if (a->nwords > b->nwords)
return false;
@@ -399,8 +410,8 @@ bms_subset_compare(const Bitmapset *a, const Bitmapset *b)
int shortlen;
int i;
- Assert(a == NULL || a->words[a->nwords - 1] != 0);
- Assert(b == NULL || b->words[b->nwords - 1] != 0);
+ Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
+ Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
/* Handle cases where either input is NULL */
if (a == NULL)
@@ -411,6 +422,9 @@ bms_subset_compare(const Bitmapset *a, const Bitmapset *b)
}
if (b == NULL)
return BMS_SUBSET2;
+
+ Assert(IsA(a, Bitmapset) && IsA(b, Bitmapset));
+
/* Check common words */
result = BMS_EQUAL; /* status so far */
shortlen = Min(a->nwords, b->nwords);
@@ -467,6 +481,9 @@ bms_is_member(int x, const Bitmapset *a)
elog(ERROR, "negative bitmapset member not allowed");
if (a == NULL)
return false;
+
+ Assert(IsA(a, Bitmapset));
+
wordnum = WORDNUM(x);
bitnum = BITNUM(x);
if (wordnum >= a->nwords)
@@ -495,6 +512,8 @@ bms_member_index(Bitmapset *a, int x)
if (!bms_is_member(x, a))
return -1;
+ Assert(IsA(a, Bitmapset));
+
wordnum = WORDNUM(x);
bitnum = BITNUM(x);
@@ -529,6 +548,9 @@ bms_overlap(const Bitmapset *a, const Bitmapset *b)
int shortlen;
int i;
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
+
/* Handle cases where either input is NULL */
if (a == NULL || b == NULL)
return false;
@@ -553,6 +575,8 @@ bms_overlap_list(const Bitmapset *a, const List *b)
int wordnum,
bitnum;
+ Assert(a == NULL || IsA(a, Bitmapset));
+
if (a == NULL || b == NIL)
return false;
@@ -582,8 +606,8 @@ bms_nonempty_difference(const Bitmapset *a, const Bitmapset *b)
{
int i;
- Assert(a == NULL || a->words[a->nwords - 1] != 0);
- Assert(b == NULL || b->words[b->nwords - 1] != 0);
+ Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
+ Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
/* Handle cases where either input is NULL */
if (a == NULL)
@@ -617,6 +641,9 @@ bms_singleton_member(const Bitmapset *a)
if (a == NULL)
elog(ERROR, "bitmapset is empty");
+
+ Assert(IsA(a, Bitmapset));
+
nwords = a->nwords;
wordnum = 0;
do
@@ -657,6 +684,7 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
if (a == NULL)
return false;
+ Assert(IsA(a, Bitmapset));
nwords = a->nwords;
wordnum = 0;
do
@@ -690,6 +718,7 @@ bms_num_members(const Bitmapset *a)
if (a == NULL)
return 0;
+ Assert(IsA(a, Bitmapset));
nwords = a->nwords;
wordnum = 0;
do
@@ -717,6 +746,7 @@ bms_membership(const Bitmapset *a)
if (a == NULL)
return BMS_EMPTY_SET;
+ Assert(IsA(a, Bitmapset));
nwords = a->nwords;
wordnum = 0;
do
@@ -759,6 +789,7 @@ bms_add_member(Bitmapset *a, int x)
elog(ERROR, "negative bitmapset member not allowed");
if (a == NULL)
return bms_make_singleton(x);
+ Assert(IsA(a, Bitmapset));
wordnum = WORDNUM(x);
bitnum = BITNUM(x);
@@ -799,6 +830,9 @@ bms_del_member(Bitmapset *a, int x)
elog(ERROR, "negative bitmapset member not allowed");
if (a == NULL)
return NULL;
+
+ Assert(IsA(a, Bitmapset));
+
wordnum = WORDNUM(x);
bitnum = BITNUM(x);
@@ -839,6 +873,9 @@ bms_add_members(Bitmapset *a, const Bitmapset *b)
int otherlen;
int i;
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
+
/* Handle cases where either input is NULL */
if (a == NULL)
return bms_copy(b);
@@ -884,6 +921,8 @@ bms_add_range(Bitmapset *a, int lower, int upper)
ushiftbits,
wordnum;
+ Assert(a == NULL || IsA(a, Bitmapset));
+
/* do nothing if nothing is called for, without further checking */
if (upper < lower)
return a;
@@ -954,6 +993,9 @@ bms_int_members(Bitmapset *a, const Bitmapset *b)
int shortlen;
int i;
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
+
/* Handle cases where either input is NULL */
if (a == NULL)
return NULL;
@@ -994,8 +1036,8 @@ bms_del_members(Bitmapset *a, const Bitmapset *b)
{
int i;
- Assert(a == NULL || a->words[a->nwords - 1] != 0);
- Assert(b == NULL || b->words[b->nwords - 1] != 0);
+ Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
+ Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
/* Handle cases where either input is NULL */
if (a == NULL)
@@ -1055,6 +1097,9 @@ bms_join(Bitmapset *a, Bitmapset *b)
int otherlen;
int i;
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
+
/* Handle cases where either input is NULL */
if (a == NULL)
return b;
@@ -1109,6 +1154,8 @@ bms_next_member(const Bitmapset *a, int prevbit)
int wordnum;
bitmapword mask;
+ Assert(a == NULL || IsA(a, Bitmapset));
+
if (a == NULL)
return -2;
nwords = a->nwords;
@@ -1168,6 +1215,8 @@ bms_prev_member(const Bitmapset *a, int prevbit)
int ushiftbits;
bitmapword mask;
+ Assert(a == NULL || IsA(a, Bitmapset));
+
/*
* If set is NULL or if there are no more bits to the right then we've
* nothing to do.
--
2.39.3 (Apple Git-145)
[application/octet-stream] 0002-REALLOCATE_BITMAPSETS-manual-compile-time-option-v2.patch (5.9K, ../CAPpHfdtLgCryACcrmLv=Koq9rAB3=tr5y9D84dGgvUhSCvjzjg@mail.gmail.com/4-0002-REALLOCATE_BITMAPSETS-manual-compile-time-option-v2.patch)
download | inline diff:
From a04499b50ac50820184e019b3d329443d55aa39a Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sun, 19 Nov 2023 01:03:12 +0200
Subject: [PATCH 2/3] REALLOCATE_BITMAPSETS manual compile-time option
This option forces each bitmapset modification to reallocate bitmapset. This
is useful for debugging hangling pointers to bitmapset's.
Discussion: https://postgr.es/m/CAMbWs4_wJthNtYBL+SsebpgF-5L2r5zFFk6xYbS0A78GKOTFHw@mail.gmail.com
---
src/backend/nodes/bitmapset.c | 77 ++++++++++++++++++++++++++++++++++
src/include/pg_config_manual.h | 6 +++
2 files changed, 83 insertions(+)
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 1627922ef7a..e13ecaa155d 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -263,6 +263,7 @@ bms_intersect(const Bitmapset *a, const Bitmapset *b)
/* Handle cases where either input is NULL */
if (a == NULL || b == NULL)
return NULL;
+
/* Identify shorter and longer input; copy the shorter one */
if (a->nwords <= b->nwords)
{
@@ -798,8 +799,15 @@ bms_add_member(Bitmapset *a, int x)
{
int oldnwords = a->nwords;
int i;
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(wordnum + 1));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+#else
a = (Bitmapset *) repalloc(a, BITMAPSET_SIZE(wordnum + 1));
+#endif
a->nwords = wordnum + 1;
/* zero out the enlarged portion */
i = oldnwords;
@@ -808,6 +816,16 @@ bms_add_member(Bitmapset *a, int x)
a->words[i] = 0;
} while (++i < a->nwords);
}
+#ifdef REALLOCATE_BITMAPSETS
+ else
+ {
+ Bitmapset *tmp = a;
+
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+ }
+#endif
a->words[wordnum] |= ((bitmapword) 1 << bitnum);
return a;
@@ -825,6 +843,9 @@ bms_del_member(Bitmapset *a, int x)
{
int wordnum,
bitnum;
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+#endif
if (x < 0)
elog(ERROR, "negative bitmapset member not allowed");
@@ -836,6 +857,12 @@ bms_del_member(Bitmapset *a, int x)
wordnum = WORDNUM(x);
bitnum = BITNUM(x);
+#ifdef REALLOCATE_BITMAPSETS
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+#endif
+
/* member can't exist. Return 'a' unmodified */
if (unlikely(wordnum >= a->nwords))
return a;
@@ -889,6 +916,13 @@ bms_add_members(Bitmapset *a, const Bitmapset *b)
}
else
{
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+#endif
result = a;
other = b;
}
@@ -941,9 +975,16 @@ bms_add_range(Bitmapset *a, int lower, int upper)
{
int oldnwords = a->nwords;
int i;
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(uwordnum + 1));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+#else
/* ensure we have enough words to store the upper bit */
a = (Bitmapset *) repalloc(a, BITMAPSET_SIZE(uwordnum + 1));
+#endif
a->nwords = uwordnum + 1;
/* zero out the enlarged portion */
i = oldnwords;
@@ -992,6 +1033,12 @@ bms_int_members(Bitmapset *a, const Bitmapset *b)
int lastnonzero;
int shortlen;
int i;
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+#endif
+
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
Assert(a == NULL || IsA(a, Bitmapset));
Assert(b == NULL || IsA(b, Bitmapset));
@@ -1004,6 +1051,13 @@ bms_int_members(Bitmapset *a, const Bitmapset *b)
pfree(a);
return NULL;
}
+
+#ifdef REALLOCATE_BITMAPSETS
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+#endif
+
/* Intersect b into a; we need never copy */
shortlen = Min(a->nwords, b->nwords);
lastnonzero = -1;
@@ -1035,6 +1089,9 @@ Bitmapset *
bms_del_members(Bitmapset *a, const Bitmapset *b)
{
int i;
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+#endif
Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));
Assert(b == NULL || (IsA(b, Bitmapset) && b->words[b->nwords - 1] != 0));
@@ -1044,6 +1101,13 @@ bms_del_members(Bitmapset *a, const Bitmapset *b)
return NULL;
if (b == NULL)
return a;
+
+#ifdef REALLOCATE_BITMAPSETS
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+#endif
+
/* Remove b's bits from a; we need never copy */
if (a->nwords > b->nwords)
{
@@ -1096,6 +1160,12 @@ bms_join(Bitmapset *a, Bitmapset *b)
Bitmapset *other;
int otherlen;
int i;
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+#endif
+
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
Assert(a == NULL || IsA(a, Bitmapset));
Assert(b == NULL || IsA(b, Bitmapset));
@@ -1105,6 +1175,13 @@ bms_join(Bitmapset *a, Bitmapset *b)
return b;
if (b == NULL)
return a;
+
+#ifdef REALLOCATE_BITMAPSETS
+ a = (Bitmapset *) palloc(BITMAPSET_SIZE(tmp->nwords));
+ memcpy(a, tmp, BITMAPSET_SIZE(tmp->nwords));
+ pfree(tmp);
+#endif
+
/* Identify shorter and longer input; use longer one as result */
if (a->nwords < b->nwords)
{
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 8a6e67a445d..16c383ba7f7 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -335,6 +335,12 @@
*/
/* #define COPY_PARSE_PLAN_TREES */
+/*
+ * Define this to force Bitmapset reallocation on each modification. Helps
+ * to find hangling pointers to Bitmapset's.
+ */
+/* #define REALLOCATE_BITMAPSETS */
+
/*
* Define this to force all parse and plan trees to be passed through
* outfuncs.c/readfuncs.c, to facilitate catching errors and omissions in
--
2.39.3 (Apple Git-145)
view thread (5+ messages)
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]
Subject: Re: Assert failure on 'list_member_ptr(rel->joininfo, restrictinfo)'
In-Reply-To: <CAPpHfdtLgCryACcrmLv=Koq9rAB3=tr5y9D84dGgvUhSCvjzjg@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