Thread: Changed default ordering in tables

Aloha!

In  v1.14 tables are opened with ORDER BY $pkey DESC.
I wonder if descending order ist intended. It used to be the other way round and, as far as I am concerned, that was
justfine in most cases. 
We have the new feature "View Data" .. "View top / last 100 rows" anyway. No need to change the default behavior?

--
Erwin Brandstetter

Hi Erwin

On Tue, Jul 26, 2011 at 4:47 PM, Erwin Brandstetter
<brandstetter@falter.at> wrote:
> Aloha!
>
> In  v1.14 tables are opened with ORDER BY $pkey DESC.
> I wonder if descending order ist intended. It used to be the other way round
> and, as far as I am concerned, that was just fine in most cases.
> We have the new feature "View Data" .. "View top / last 100 rows" anyway. No
> need to change the default behavior?

The default is determined like this:

       orderBy = table->GetQuotedPrimaryKey();
       if (orderBy.IsEmpty() && hasOids)
            orderBy = wxT("oid");
       if (!orderBy.IsEmpty())
      {
           if (pkAscending)
                orderBy += wxT(" ASC");
           else
              orderBy += wxT(" DESC");
      }

Essentially, we try to follow the ordering in the index.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 26.07.2011 18:12, Dave Page wrote:
> Hi Erwin
>
> On Tue, Jul 26, 2011 at 4:47 PM, Erwin Brandstetter
> <brandstetter@falter.at>  wrote:
>> Aloha!
>>
>> In  v1.14 tables are opened with ORDER BY $pkey DESC.
>> I wonder if descending order ist intended. It used to be the other way round
>> and, as far as I am concerned, that was just fine in most cases.
>> We have the new feature "View Data" .. "View top / last 100 rows" anyway. No
>> need to change the default behavior?
> The default is determined like this:
>
>         orderBy = table->GetQuotedPrimaryKey();
>         if (orderBy.IsEmpty()&&  hasOids)
>              orderBy = wxT("oid");
>         if (!orderBy.IsEmpty())
>        {
>             if (pkAscending)
>                  orderBy += wxT(" ASC");
>             else
>                orderBy += wxT(" DESC");
>        }
>
> Essentially, we try to follow the ordering in the index.
>

Aloha Dave!

Fair enough. However, the following test-case shows the opposite effect in pgAdmin:

 > By default, B-tree indexes store their entries in ascending order with nulls last.
http://www.postgresql.org/docs/9.0/interactive/indexes-ordering.html

CREATE TABLE test(test_id integer primary key, test text);
INSERT INTO test VALUES (1, 'top'), (2, 'middle'), (3, 'bottom')
-- Now open the table in the browser of pgAdmin 1.14 Beta 3 (sorts DESC; incorrect)
-- Compare this with the behaviour in pgAdmin 1.12 (sorts ASC; correct)

Also, there are key types that do not sort. I quote the documementation:
 > Of the index types currently supported by PostgreSQL, only B-tree can produce sorted output — the other index types
returnmatching rows in an  
unspecified, implementation-dependent order.

Is it safe to assume descending order if pkAscending is not true? Not sure what "IsEmpty" implies exactly in the code
..

--
Erwin Brandstetter

On Tue, Jul 26, 2011 at 7:13 PM, Erwin Brandstetter
<brandstetter@falter.at> wrote:
> On 26.07.2011 18:12, Dave Page wrote:
>>
>> Hi Erwin
>>
>> On Tue, Jul 26, 2011 at 4:47 PM, Erwin Brandstetter
>> <brandstetter@falter.at>  wrote:
>>>
>>> Aloha!
>>>
>>> In  v1.14 tables are opened with ORDER BY $pkey DESC.
>>> I wonder if descending order ist intended. It used to be the other way
>>> round
>>> and, as far as I am concerned, that was just fine in most cases.
>>> We have the new feature "View Data" .. "View top / last 100 rows" anyway.
>>> No
>>> need to change the default behavior?
>>
>> The default is determined like this:
>>
>>        orderBy = table->GetQuotedPrimaryKey();
>>        if (orderBy.IsEmpty()&&  hasOids)
>>             orderBy = wxT("oid");
>>        if (!orderBy.IsEmpty())
>>       {
>>            if (pkAscending)
>>                 orderBy += wxT(" ASC");
>>            else
>>               orderBy += wxT(" DESC");
>>       }
>>
>> Essentially, we try to follow the ordering in the index.

Actually, no, we don't (sorry). The flag is simply set by the caller
to do FIRST/LAST 100 rows. Otherwise, it defaults to true.

>
> Fair enough. However, the following test-case shows the opposite effect in
> pgAdmin:
>
>> By default, B-tree indexes store their entries in ascending order with
>> nulls last.
> http://www.postgresql.org/docs/9.0/interactive/indexes-ordering.html
>
> CREATE TABLE test(test_id integer primary key, test text);
> INSERT INTO test VALUES (1, 'top'), (2, 'middle'), (3, 'bottom')
> -- Now open the table in the browser of pgAdmin 1.14 Beta 3 (sorts DESC;
> incorrect)
> -- Compare this with the behaviour in pgAdmin 1.12 (sorts ASC; correct)

It works correctly for me.

> Also, there are key types that do not sort. I quote the documementation:
>> Of the index types currently supported by PostgreSQL, only B-tree can
>> produce sorted output — the other index types return matching rows in an
>> unspecified, implementation-dependent order.

Right, but a primary key is a UNIQUE + NOT NULL (and a flag in the
catalogs). Unique indexes are always B-Trees in Postgres.

> Is it safe to assume descending order if pkAscending is not true? Not sure
> what "IsEmpty" implies exactly in the code ..

The IsEmpty bit is testing to see if the user has set any explicit
ordering on the Sort/Filter dialogue - if so, that takes precedence.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 26.07.2011 22:29, Dave Page wrote:
> On Tue, Jul 26, 2011 at 7:13 PM, Erwin Brandstetter
> <brandstetter@falter.at>  wrote:
>> On 26.07.2011 18:12, Dave Page wrote:
>>> Hi Erwin
>>>
>>> On Tue, Jul 26, 2011 at 4:47 PM, Erwin Brandstetter
>>> <brandstetter@falter.at>    wrote:
>>>> Aloha!
>>>>
>>>> In  v1.14 tables are opened with ORDER BY $pkey DESC.
>>>> I wonder if descending order ist intended. It used to be the other way
>>>> round
>>>> and, as far as I am concerned, that was just fine in most cases.
>>>> We have the new feature "View Data" .. "View top / last 100 rows" anyway.
>>>> No
>>>> need to change the default behavior?
>>> The default is determined like this:
>>>
>>>         orderBy = table->GetQuotedPrimaryKey();
>>>         if (orderBy.IsEmpty()&&    hasOids)
>>>              orderBy = wxT("oid");
>>>         if (!orderBy.IsEmpty())
>>>        {
>>>             if (pkAscending)
>>>                  orderBy += wxT(" ASC");
>>>             else
>>>                orderBy += wxT(" DESC");
>>>        }
>>>
>>> Essentially, we try to follow the ordering in the index.
> Actually, no, we don't (sorry). The flag is simply set by the caller
> to do FIRST/LAST 100 rows. Otherwise, it defaults to true.
>
>> Fair enough. However, the following test-case shows the opposite effect in
>> pgAdmin:
>>
>>> By default, B-tree indexes store their entries in ascending order with
>>> nulls last.
>> http://www.postgresql.org/docs/9.0/interactive/indexes-ordering.html
>>
>> CREATE TABLE test(test_id integer primary key, test text);
>> INSERT INTO test VALUES (1, 'top'), (2, 'middle'), (3, 'bottom')
>> -- Now open the table in the browser of pgAdmin 1.14 Beta 3 (sorts DESC;
>> incorrect)
>> -- Compare this with the behaviour in pgAdmin 1.12 (sorts ASC; correct)
> It works correctly for me.
>
>> Also, there are key types that do not sort. I quote the documementation:
>>> Of the index types currently supported by PostgreSQL, only B-tree can
>>> produce sorted output — the other index types return matching rows in an
>>> unspecified, implementation-dependent order.
> Right, but a primary key is a UNIQUE + NOT NULL (and a flag in the
> catalogs). Unique indexes are always B-Trees in Postgres.
>
>> Is it safe to assume descending order if pkAscending is not true? Not sure
>> what "IsEmpty" implies exactly in the code ..
> The IsEmpty bit is testing to see if the user has set any explicit
> ordering on the Sort/Filter dialogue - if so, that takes precedence.

That may be the key here. When I open the simple test table from above in v1.14 I get descending order. On opening the
Sort/Filterdialog it see an  
explicit ORDER BY test_id DESCENDING - which I did not set.
Once I delete that, the sort order falls back to ascending - as it should to begin with.
However, after reopening the table explicit descending order is back.

Interesting that you cannot reproduce the effect. Ar you on Apple?
I am tested with all combinations of pgAdmin 1.12 & 1.14 Beta 3 on Win XP Pro and postgres 9.0.4 and 8.4.8 on Debian
Squeeze.

Playing with the new Features "View Data" .. "View top / last 100 rows" has no lasting side effects on this problem.

--
Erwin Brandstetter

On Tue, Jul 26, 2011 at 9:53 PM, Erwin Brandstetter
<brandstetter@falter.at> wrote:
> That may be the key here. When I open the simple test table from above in
> v1.14 I get descending order. On opening the Sort/Filter dialog it see an
> explicit ORDER BY test_id DESCENDING - which I did not set.
> Once I delete that, the sort order falls back to ascending - as it should to
> begin with.
> However, after reopening the table explicit descending order is back.

I see the same... except it's defaulting to ASC.

> Interesting that you cannot reproduce the effect. Ar you on Apple?
> I am tested with all combinations of pgAdmin 1.12 & 1.14 Beta 3 on Win XP
> Pro and postgres 9.0.4 and 8.4.8 on Debian Squeeze.

Tested on Mac and Windows 7.

> Playing with the new Features "View Data" .. "View top / last 100 rows" has
> no lasting side effects on this problem.

Very odd. Anyone else able to reproduce this?



--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 26.07.2011 23:20, Dave Page wrote:
> On Tue, Jul 26, 2011 at 9:53 PM, Erwin Brandstetter
> <brandstetter@falter.at>  wrote:
>> That may be the key here. When I open the simple test table from above in
>> v1.14 I get descending order. On opening the Sort/Filter dialog it see an
>> explicit ORDER BY test_id DESCENDING - which I did not set.
>> Once I delete that, the sort order falls back to ascending - as it should to
>> begin with.
>> However, after reopening the table explicit descending order is back.
> I see the same... except it's defaulting to ASC.
>
>> Interesting that you cannot reproduce the effect. Ar you on Apple?
>> I am tested with all combinations of pgAdmin 1.12&  1.14 Beta 3 on Win XP
>> Pro and postgres 9.0.4 and 8.4.8 on Debian Squeeze.
> Tested on Mac and Windows 7.
>
>> Playing with the new Features "View Data" .. "View top / last 100 rows" has
>> no lasting side effects on this problem.
> Very odd. Anyone else able to reproduce this?
>

Yeah, me. ;) On another Win XP machine ...


--

Erwin Brandstetter

On 29.07.2011 04:57, Erwin Brandstetter wrote:
> On 26.07.2011 23:20, Dave Page wrote:
>> On Tue, Jul 26, 2011 at 9:53 PM, Erwin Brandstetter
>> <brandstetter@falter.at>  wrote:
>>> That may be the key here. When I open the simple test table from
>>> above in
>>> v1.14 I get descending order. On opening the Sort/Filter dialog it
>>> see an
>>> explicit ORDER BY test_id DESCENDING - which I did not set.
>>> Once I delete that, the sort order falls back to ascending - as it
>>> should to
>>> begin with.

Actually, the sort order falls back to random, which happens to be
ascending by chance in the test case.

I ran more tests. (on Windows XP Pro) The results for multiple-row pkeys
might interest you:
Only the sort ordering for the _last_ index field is reversed.
Try these test case with 1,2 and three pkey columns:

     CREATE TABLE test1(id1 int PRIMARY KEY);
     INSERT INTO test1 VALUES (1),(2),(3),(4),(5),(6);

     CREATE TABLE test2(id1 int, id2 int, CONSTRAINT test2_pk PRIMARY
KEY (id1, id2));
     INSERT INTO test2 VALUES
(1,1),(1,2),(1,3),(2,1),(2,2),(2,3),(3,1),(3,2),(3,3);

     CREATE TABLE test3(id1 int, id2 int, id3 int, CONSTRAINT test3_pk
PRIMARY KEY (id1, id2, id3));
     INSERT INTO test3 VALUES
(1,1,1),(1,1,2),(1,1,3),(1,2,1),(1,2,2),(1,2,3),(1,3,1),(1,3,2),(1,3,3);

If you open these tables, the server gets:
     SELECT * FROM public.test1 ORDER BY id1 DESC
     SELECT * FROM public.test2 ORDER BY id1, id2 DESC
     SELECT * FROM public.test3 ORDER BY id1, id2, id3 DESC

IOW, a single ' DESC' is appended to the SELECT, which is a bug in any case.
If DESC was intended, it would apply on all three columns.

It puzzles me, that Dave cannot reproduce it on his Mac. Can sombody
else give it a shot and report back?
Do you want me to open a ticket?


--

Erwin Brandstetter

On 29.07.2011 07:10, Erwin Brandstetter wrote:
> On 29.07.2011 04:57, Erwin Brandstetter wrote:
>> On 26.07.2011 23:20, Dave Page wrote:
>>> On Tue, Jul 26, 2011 at 9:53 PM, Erwin Brandstetter
>>> <brandstetter@falter.at>  wrote:
>>>> That may be the key here. When I open the simple test table from
>>>> above in
>>>> v1.14 I get descending order. On opening the Sort/Filter dialog it
>>>> see an
>>>> explicit ORDER BY test_id DESCENDING - which I did not set.
>>>> Once I delete that, the sort order falls back to ascending - as it
>>>> should to
>>>> begin with.
>
> Actually, the sort order falls back to random, which happens to be
> ascending by chance in the test case.
>
> I ran more tests. (on Windows XP Pro) The results for multiple-row
> pkeys might interest you:
> Only the sort ordering for the _last_ index field is reversed.
> Try these test case with 1,2 and three pkey columns:
>
>     CREATE TABLE test1(id1 int PRIMARY KEY);
>     INSERT INTO test1 VALUES (1),(2),(3),(4),(5),(6);
>
>     CREATE TABLE test2(id1 int, id2 int, CONSTRAINT test2_pk PRIMARY
> KEY (id1, id2));
>     INSERT INTO test2 VALUES
> (1,1),(1,2),(1,3),(2,1),(2,2),(2,3),(3,1),(3,2),(3,3);
>
>     CREATE TABLE test3(id1 int, id2 int, id3 int, CONSTRAINT test3_pk
> PRIMARY KEY (id1, id2, id3));
>     INSERT INTO test3 VALUES
> (1,1,1),(1,1,2),(1,1,3),(1,2,1),(1,2,2),(1,2,3),(1,3,1),(1,3,2),(1,3,3);
>
> If you open these tables, the server gets:
>     SELECT * FROM public.test1 ORDER BY id1 DESC
>     SELECT * FROM public.test2 ORDER BY id1, id2 DESC
>     SELECT * FROM public.test3 ORDER BY id1, id2, id3 DESC
>
> IOW, a single ' DESC' is appended to the SELECT, which is a bug in any
> case.
> If DESC was intended, it would apply on all three columns.
>
> It puzzles me, that Dave cannot reproduce it on his Mac. Can sombody
> else give it a shot and report back?
> Do you want me to open a ticket?

Actually, the feature "View data" .. "Last 100 rows" is broken for
multi-column pkeys. It send a query of the form (same as above):
     SELECT * FROM public.test3 ORDER BY id1, id2, id3 DESC
Where it should be:
     SELECT * FROM public.test3 ORDER BY id1 DESC, id2 DESC, id3 DESC

So, that's two distinct issue:
- "Last 100 rows" broken for multi-column pkeys.
- Default sort order DESC for edit grid, where it should be ASC.

--

Erwin Brandstetter

On Fri, Jul 29, 2011 at 9:14 AM, Erwin Brandstetter
<brandstetter@falter.at> wrote:
> So, that's two distinct issue:
> - "Last 100 rows" broken for multi-column pkeys.
> - Default sort order DESC for edit grid, where it should be ASC.

That one I can reproduce, and fix. Patch attached.

Ashesh, can you review and test please? We're getting close to release
so I want to be extra careful.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, 2011-07-29 at 10:33 +0100, Dave Page wrote:
> On Fri, Jul 29, 2011 at 9:14 AM, Erwin Brandstetter
> <brandstetter@falter.at> wrote:
> > So, that's two distinct issue:
> > - "Last 100 rows" broken for multi-column pkeys.
> > - Default sort order DESC for edit grid, where it should be ASC.
>
> That one I can reproduce, and fix. Patch attached.
>
> Ashesh, can you review and test please? We're getting close to release
> so I want to be extra careful.
>

I checked your patch. Seems good to me. So I applied it. I also applied
another patch to fix Erwin's bug (missing initialization of the
pkAscending variable).


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

Thanks. I thought i checked for the var init. Guess i missed a code path.

On Saturday, August 6, 2011, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> On Fri, 2011-07-29 at 10:33 +0100, Dave Page wrote:
>> On Fri, Jul 29, 2011 at 9:14 AM, Erwin Brandstetter
>> <brandstetter@falter.at> wrote:
>> > So, that's two distinct issue:
>> > - "Last 100 rows" broken for multi-column pkeys.
>> > - Default sort order DESC for edit grid, where it should be ASC.
>>
>> That one I can reproduce, and fix. Patch attached.
>>
>> Ashesh, can you review and test please? We're getting close to release
>> so I want to be extra careful.
>>
>
> I checked your patch. Seems good to me. So I applied it. I also applied
> another patch to fix Erwin's bug (missing initialization of the
> pkAscending variable).
>
>
> --
> Guillaume
>  http://blog.guillaume.lelarge.info
>  http://www.dalibo.com
>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Thanks to both of you!

Looking forward to the next release!

--
Erwin Brandstetter



On 06.08.2011 14:49, Guillaume Lelarge wrote:
> On Fri, 2011-07-29 at 10:33 +0100, Dave Page wrote:
>> On Fri, Jul 29, 2011 at 9:14 AM, Erwin Brandstetter
>> <brandstetter@falter.at>  wrote:
>>> So, that's two distinct issue:
>>> - "Last 100 rows" broken for multi-column pkeys.
>>> - Default sort order DESC for edit grid, where it should be ASC.
>> That one I can reproduce, and fix. Patch attached.
>>
>> Ashesh, can you review and test please? We're getting close to release
>> so I want to be extra careful.
>>
> I checked your patch. Seems good to me. So I applied it. I also applied
> another patch to fix Erwin's bug (missing initialization of the
> pkAscending variable).
>
>