Thread: BUG #4921: ltree @> ltree[] operator shouldn't fail if ltree[] is empty

The following bug has been logged online:

Bug reference:      4921
Logged by:          Alan Pinstein
Email address:      apinstein@mac.com
PostgreSQL version: 8.3.6
Operating system:   linux/centos 5.3
Description:        ltree @> ltree[] operator shouldn't fail if ltree[] is
empty
Details:

The following query:

        select

feature_id,hierarchy,description,category,ok_community,ok_property,ok_land,o
k_structure,ok_level,ok_room,
            (select count(*) from feature where f.hierarchy =
subpath(hierarchy,0,-1) and ok_property = true) as count
        from
            feature f
        where
            ok_property = true and hierarchy @> ARRAY(select hierarchy from
feature where description ilike '%pool%this%') and nlevel(hierarchy) = 1
         order by hierarchy asc

NOTES:
- hierarchy is an ltree in the feature table
- this query finds all root items in tree which contain any nodes whose
description matches "%pool%this%"
- the subquery returns 0 rows (there are no matching items in the error case
being reported)

EXPECTED BEHAVIOR:
- return 0 rows

ACTUAL BEHAVIOR:
ERROR:  array must be one-dimensional
Possibly from:
https://projects.commandprompt.com/public/replicator/browser/trunk/contrib/l
tree/_ltree_op.c?rev=1905 line 46

NOTES:
This query worked in 8.1.x and started failing in 8.3.6 (only 2 versions I
tested).
I was able to hack around the issue with:

... hierarchy @> nullif(ARRAY(select hierarchy from feature where
description ilike '%pool%'),'{}') ...

Thank you very much for ltree, it rocks.

Feel free to contact me if you have further questions.

Alan

"Alan Pinstein" <apinstein@mac.com> writes:
>   ... hierarchy @> ARRAY(select hierarchy from
> feature where description ilike '%pool%this%') ...

> EXPECTED BEHAVIOR:
> - return 0 rows

> ACTUAL BEHAVIOR:
> ERROR:  array must be one-dimensional
> Possibly from:
> https://projects.commandprompt.com/public/replicator/browser/trunk/contrib/ltree/_ltree_op.c?rev=1905 line 46

> NOTES:
> This query worked in 8.1.x and started failing in 8.3.6 (only 2 versions I
> tested).

Hmm.  ltree has always had that ARR_NDIM == 1 check.  I think the reason
the behavior changed is that ARRAY(SELECT ...) used to return a NULL for
zero rows, and now it returns an empty (zero-dimensional) array.

I can see two reasonable ways to address this:

* Change the ltree test to reject only ARR_NDIM > 1.

* Drop the ARR_NDIM check altogether, and let it search any sort of
array.

I'm leaning to #2 myself.  However, there are probably other places with
the same kind of issue, and in some of them it might make more sense to
reject multidimensional arrays.

            regards, tom lane

Alan Pinstein <apinstein@mac.com> writes:
> The real solution might be to just convert a 0-dim array into "null"
> or equivalent and still assert error if dims >= 2?

That's my alternative #1.  The question is whether there's any real
point in rejecting multi-dimensional arrays here, rather than just
searching all the elements regardless of the array shape.

            regards, tom lane

> Hmm.  ltree has always had that ARR_NDIM == 1 check.  I think the
> reason
> the behavior changed is that ARRAY(SELECT ...) used to return a NULL
> for
> zero rows, and now it returns an empty (zero-dimensional) array.

Ah OK that makes sense, especially given the "hack" I used as a
workaround, which effectively emulates the old behavior.

> I can see two reasonable ways to address this:
>
> * Change the ltree test to reject only ARR_NDIM > 1.
>
> * Drop the ARR_NDIM check altogether, and let it search any sort of
> array.
>
> I'm leaning to #2 myself.  However, there are probably other places
> with
> the same kind of issue, and in some of them it might make more sense
> to
> reject multidimensional arrays.

There may be a third option; it seems to me that the assert is there
more to stop unexpected behavior with arrays of dims of 2 or more.

The real solution might be to just convert a 0-dim array into "null"
or equivalent and still assert error if dims >= 2?

Alan

Yeah, and I don't feel I know enough to answer that.

Thanks for responding! Good luck with your decision.

Regards,

Alan

On Jul 15, 2009, at 11:33 AM, Tom Lane wrote:

> Alan Pinstein <apinstein@mac.com> writes:
>> The real solution might be to just convert a 0-dim array into "null"
>> or equivalent and still assert error if dims >= 2?
>
> That's my alternative #1.  The question is whether there's any real
> point in rejecting multi-dimensional arrays here, rather than just
> searching all the elements regardless of the array shape.
>
>             regards, tom lane

Was this ever addressed?

---------------------------------------------------------------------------

Tom Lane wrote:
> "Alan Pinstein" <apinstein@mac.com> writes:
> >   ... hierarchy @> ARRAY(select hierarchy from
> > feature where description ilike '%pool%this%') ...
>
> > EXPECTED BEHAVIOR:
> > - return 0 rows
>
> > ACTUAL BEHAVIOR:
> > ERROR:  array must be one-dimensional
> > Possibly from:
> > https://projects.commandprompt.com/public/replicator/browser/trunk/contrib/ltree/_ltree_op.c?rev=1905 line 46
>
> > NOTES:
> > This query worked in 8.1.x and started failing in 8.3.6 (only 2 versions I
> > tested).
>
> Hmm.  ltree has always had that ARR_NDIM == 1 check.  I think the reason
> the behavior changed is that ARRAY(SELECT ...) used to return a NULL for
> zero rows, and now it returns an empty (zero-dimensional) array.
>
> I can see two reasonable ways to address this:
>
> * Change the ltree test to reject only ARR_NDIM > 1.
>
> * Drop the ARR_NDIM check altogether, and let it search any sort of
> array.
>
> I'm leaning to #2 myself.  However, there are probably other places with
> the same kind of issue, and in some of them it might make more sense to
> reject multidimensional arrays.
>
>             regards, tom lane
>
> --
> Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
  + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> Was this ever addressed?

No, it doesn't look like the code's been changed.  I was looking for
some comments about which to do:

>> I can see two reasonable ways to address this:
>>
>> * Change the ltree test to reject only ARR_NDIM > 1.
>>
>> * Drop the ARR_NDIM check altogether, and let it search any sort of
>> array.
>>
>> I'm leaning to #2 myself.  However, there are probably other places with
>> the same kind of issue, and in some of them it might make more sense to
>> reject multidimensional arrays.

Thoughts?

            regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Was this ever addressed?
>
> No, it doesn't look like the code's been changed.  I was looking for
> some comments about which to do:
>
> >> I can see two reasonable ways to address this:
> >>
> >> * Change the ltree test to reject only ARR_NDIM > 1.
> >>
> >> * Drop the ARR_NDIM check altogether, and let it search any sort of
> >> array.
> >>
> >> I'm leaning to #2 myself.  However, there are probably other places with
> >> the same kind of issue, and in some of them it might make more sense to
> >> reject multidimensional arrays.
>
> Thoughts?

Do something.  ;-)  LOL

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
  + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>>> I can see two reasonable ways to address this:
>>>
>>> * Change the ltree test to reject only ARR_NDIM > 1.
>>>
>>> * Drop the ARR_NDIM check altogether, and let it search any sort of
>>> array.
>>>
>>> I'm leaning to #2 myself.  However, there are probably other places with
>>> the same kind of issue, and in some of them it might make more sense to
>>> reject multidimensional arrays.
>>
>> Thoughts?

> Do something.  ;-)  LOL

I'll drop the check then.

            regards, tom lane

I wrote:
>>> I can see two reasonable ways to address this:
>>>
>>> * Change the ltree test to reject only ARR_NDIM > 1.
>>>
>>> * Drop the ARR_NDIM check altogether, and let it search any sort of
>>> array.
>>>
>>> I'm leaning to #2 myself.  However, there are probably other places with
>>> the same kind of issue, and in some of them it might make more sense to
>>> reject multidimensional arrays.

After looking closer, I see that there are seven different occurrences
of this coding pattern in contrib/ltree.  They all look to be sane for
zero-element arrays but I'm hesitant to decide that they should all
allow multidimensionals.  So I'll go with fix #1 instead.

I don't see any other trouble spots elsewhere.  There are other tests
that require ARR_NDIM() == 1, but they are in contexts that will reject
empty arrays anyway, so I don't feel a need to change them.

            regards, tom lane