Re: Bug in intarray?

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

Guillaume Lelarge <guillaume(at)lelarge(dot)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

On Thu, 2012-02-16 at 19:27 -0500, Tom Lane wrote:
> Guillaume Lelarge <guillaume(at)lelarge(dot)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