Hello and sorry for the big delay in reply!
On 04.05.2023 15:22, Tom Lane wrote:
AFAICS the change you propose would serve only to mask bugs.
Yes, it's a bad decision.
Under what circumstances would you be trying to inject INDEX_VAR
into a nullingrel set? Only outer-join relids should ever appear there.
The thing is that i don't try to push INDEX_VAR into a nullingrel set at all,
i just try to replace the existing rt_index that equals to INDEX_VAR in Var
nodes with
the defined positive indexes by using ChangeVarNodes_walker() function call. It
checks
if the nullingrel contains the existing rt_index and does it need to be updated
too.
It calls bms_is_member() for that, but the last immediately throws an error
if the value to be checked is negative like INDEX_VAR.
But we are not trying to corrupt the existing nullingrel with this bad index,
so it doesn't seem like an serious error.
And this index certainly cannot be a member of the Bitmapset.
Therefore it also seems better and more logical to me in the case of an index
that
cannot possibly be a member of the Bitmapset, immediately return false.
Here is a patch like that.
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
commit cc1724759b898efc703867a83d38173e4b2794b5
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date: Mon May 29 13:52:42 2023 +0300
Return false from bms_is_member() if checked value is negative.
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 7ba3cf635b..3e1db5fda2 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -446,9 +446,9 @@ bms_is_member(int x, const Bitmapset *a)
int wordnum,
bitnum;
- /* XXX better to just return false for x<0 ? */
+ /* bitmapset member cannot be negative */
if (x < 0)
- elog(ERROR, "negative bitmapset member not allowed");
+ return false;
if (a == NULL)
return false;
wordnum = WORDNUM(x);
