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


diff --git a/contrib/intarray/_int_tool.c b/contrib/intarray/_int_tool.c
index 79f018d333b1d6810aa7fd2996c158b41b0263fb..132d15316054d842593468f191eca9053846f706 100644
*** a/contrib/intarray/_int_tool.c
--- b/contrib/intarray/_int_tool.c
*************** inner_int_inter(ArrayType *a, ArrayType
*** 140,146 ****
                 *db,
                 *dr;
      int            i,
!                 j;

      if (ARRISEMPTY(a) || ARRISEMPTY(b))
          return new_intArrayType(0);
--- 140,147 ----
                 *db,
                 *dr;
      int            i,
!                 j,
!                 k;

      if (ARRISEMPTY(a) || ARRISEMPTY(b))
          return new_intArrayType(0);
*************** inner_int_inter(ArrayType *a, ArrayType
*** 152,166 ****
      r = new_intArrayType(Min(na, nb));
      dr = ARRPTR(r);

!     i = j = 0;
      while (i < na && j < nb)
      {
          if (da[i] < db[j])
              i++;
          else if (da[i] == db[j])
          {
!             if (i + j == 0 || (i + j > 0 && *(dr - 1) != db[j]))
!                 *dr++ = db[j];
              i++;
              j++;
          }
--- 153,167 ----
      r = new_intArrayType(Min(na, nb));
      dr = ARRPTR(r);

!     i = j = k = 0;
      while (i < na && j < nb)
      {
          if (da[i] < db[j])
              i++;
          else if (da[i] == db[j])
          {
!             if (k == 0 || dr[k - 1] != db[j])
!                 dr[k++] = db[j];
              i++;
              j++;
          }
*************** inner_int_inter(ArrayType *a, ArrayType
*** 168,180 ****
              j++;
      }

!     if ((dr - ARRPTR(r)) == 0)
      {
          pfree(r);
          return new_intArrayType(0);
      }
      else
!         return resize_intArrayType(r, dr - ARRPTR(r));
  }

  void
--- 169,181 ----
              j++;
      }

!     if (k == 0)
      {
          pfree(r);
          return new_intArrayType(0);
      }
      else
!         return resize_intArrayType(r, k);
  }

  void
diff --git a/contrib/intarray/expected/_int.out b/contrib/intarray/expected/_int.out
index 6ed3cc6ced096e2e97665e76ceba7fc139415029..4080b9633fe98a91861684e0d82f1297c21b91af 100644
*** a/contrib/intarray/expected/_int.out
--- b/contrib/intarray/expected/_int.out
*************** SELECT '{123,623,445}'::int[] & '{1623,6
*** 137,142 ****
--- 137,148 ----
   {623}
  (1 row)

+ SELECT '{-1,3,1}'::int[] & '{1,2}';
+  ?column?
+ ----------
+  {1}
+ (1 row)
+
  --test query_int
  SELECT '1'::query_int;
   query_int
diff --git a/contrib/intarray/sql/_int.sql b/contrib/intarray/sql/_int.sql
index b60e936dc520d8308bd16e50681f061dc86e2dfe..216c5c58d615a7cd1a5fe3714c9a5c91fb255138 100644
*** a/contrib/intarray/sql/_int.sql
--- b/contrib/intarray/sql/_int.sql
*************** SELECT '{123,623,445}'::int[] | 623;
*** 24,29 ****
--- 24,30 ----
  SELECT '{123,623,445}'::int[] | 1623;
  SELECT '{123,623,445}'::int[] | '{1623,623}';
  SELECT '{123,623,445}'::int[] & '{1623,623}';
+ SELECT '{-1,3,1}'::int[] & '{1,2}';


  --test query_int