Bug in intarray?

Started by Guillaume Lelargealmost 14 years ago3 messages
#1Guillaume Lelarge
guillaume@lelarge.info
1 attachment(s)

Hi,

On a french PostgreSQL web forum, one of our users asked about a curious
behaviour of the intarray extension.

This query:
SELECT ARRAY[-1,3,1] & ARRAY[1, 2];
should give {1} as a result.

But, on HEAD (and according to his tests, on 9.0.6 and 9.1.2), it
appears to give en empty array.

Digging on this issue, another user (Julien Rouhaud) made an interesting
comment on this line of code:

if (i + j == 0 || (i + j > 0 && *(dr - 1) != db[j]))

(line 159 of contrib/intarray/_int_tool.c, current HEAD)

Apparently, the code tries to check the current value of the right side
array with the previous value of the resulting array. Which clearly
cannot work if there is no previous value in the resulting array.

So I worked on a patch to fix this, as I think it is a bug (but I may be
wrong). Patch is attached and fixes the issue AFAICT.

Thanks.

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com

Attachments:

inner_int_inter.patchtext/x-patch; charset=UTF-8; name=inner_int_inter.patchDownload
diff --git a/contrib/intarray/_int_tool.c b/contrib/intarray/_int_tool.c
index 79f018d..4d7a1f2 100644
--- a/contrib/intarray/_int_tool.c
+++ b/contrib/intarray/_int_tool.c
@@ -159,7 +159,7 @@ inner_int_inter(ArrayType *a, ArrayType *b)
 			i++;
 		else if (da[i] == db[j])
 		{
-			if (i + j == 0 || (i + j > 0 && *(dr - 1) != db[j]))
+			if (i + j == 0 || (i + j > 0 && (dr - ARRPTR(r)) == 0) || (i + j > 0 && *(dr - 1) != db[j]))
 				*dr++ = db[j];
 			i++;
 			j++;
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Guillaume Lelarge (#1)
Re: Bug in intarray?

Guillaume Lelarge <guillaume@lelarge.info> writes:

This query:
SELECT ARRAY[-1,3,1] & ARRAY[1, 2];
should give {1} as a result.

But, on HEAD (and according to his tests, on 9.0.6 and 9.1.2), it
appears to give en empty array.

Definitely a bug, and I'll bet it goes all the way back.

Digging on this issue, another user (Julien Rouhaud) made an interesting
comment on this line of code:

if (i + j == 0 || (i + j > 0 && *(dr - 1) != db[j]))

(line 159 of contrib/intarray/_int_tool.c, current HEAD)

Apparently, the code tries to check the current value of the right side
array with the previous value of the resulting array. Which clearly
cannot work if there is no previous value in the resulting array.

So I worked on a patch to fix this, as I think it is a bug (but I may be
wrong). Patch is attached and fixes the issue AFAICT.

Yeah, this code is bogus, but it's also pretty unreadable. I think
it's better to get rid of the inconsistently-used pointer arithmetic
and the fundamentally wrong/irrelevant test on i+j, along the lines
of the attached.

regards, tom lane

#3Guillaume Lelarge
guillaume@lelarge.info
In reply to: Tom Lane (#2)
Re: Bug in intarray?

On Thu, 2012-02-16 at 19:27 -0500, Tom Lane wrote:

Guillaume Lelarge <guillaume@lelarge.info> writes:

This query:
SELECT ARRAY[-1,3,1] & ARRAY[1, 2];
should give {1} as a result.

But, on HEAD (and according to his tests, on 9.0.6 and 9.1.2), it
appears to give en empty array.

Definitely a bug, and I'll bet it goes all the way back.

Digging on this issue, another user (Julien Rouhaud) made an interesting
comment on this line of code:

if (i + j == 0 || (i + j > 0 && *(dr - 1) != db[j]))

(line 159 of contrib/intarray/_int_tool.c, current HEAD)

Apparently, the code tries to check the current value of the right side
array with the previous value of the resulting array. Which clearly
cannot work if there is no previous value in the resulting array.

So I worked on a patch to fix this, as I think it is a bug (but I may be
wrong). Patch is attached and fixes the issue AFAICT.

Yeah, this code is bogus, but it's also pretty unreadable. I think
it's better to get rid of the inconsistently-used pointer arithmetic
and the fundamentally wrong/irrelevant test on i+j, along the lines
of the attached.

Completely agree.

Thank you.

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com