db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Satheesh Bandaram <sathe...@sourcery.org>
Subject Re: About improvement of DERBY-134
Date Fri, 11 Mar 2005 21:59:20 GMT
Hi Tomohito Nakayama,

Just wanted to check how you are progressing on the patch update,
following comments by myself and Jack. I do think you are working on an
important enhancement that not only yourself but other developpers have
expressed interest in. I strongly encourage you to continue working on
this and post any questions or comments you might have. You are pretty
close to addressing all issues.

I am willing to help, if you need any, to continue taking this further.

Satheesh

TomohitoNakayama wrote:

> Hello.
> Thanks for your reviewing.
>
> About 1:
> Handling any sortKey as expression is better structure.
> A little challenging but worth for it.
> I will try.
>
> About 2:
> Uh oh.
> Bug about starting value of element indexing in ResultColumnList ....
> Test of comma separated lists of ORDER BY expressions in orderby.sql
> was needed.....
>
> About 3:
> I see.
> It seems that it is certainly needed to add test case .
>
> I will continue this issue.
> Best regards.
>
> /*
>
> Tomohito Nakayama
> tomoihto@rose.zero.ad.jp
> tomonaka@basil.ocn.ne.jp
>
> Naka
> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>
> */
> ----- Original Message ----- From: "Jack Klebanoff"
> <klebanoff-derby@sbcglobal.net>
> To: "Derby Development" <derby-dev@db.apache.org>
> Sent: Sunday, February 20, 2005 8:37 AM
> Subject: Re: About improvement of DERBY-134
>
>
>> TomohitoNakayama wrote:
>>
>>> Hello.
>>>
>>> I have put some LOOKAHEAD to sqlgrammer.jj and
>>> add some test pattern to orderby.sql.
>>>
>>> Would someone review patch please ?
>>>
>>> Best regards.
>>>
>>> /*
>>>
>>> Tomohito Nakayama
>>> tomoihto@rose.zero.ad.jp
>>> tomonaka@basil.ocn.ne.jp
>>>
>>> Naka
>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>
>>> */
>>> ----- Original Message ----- From: "TomohitoNakayama"
>>> <tomonaka@basil.ocn.ne.jp>
>>> To: "Derby Development" <derby-dev@db.apache.org>
>>> Sent: Sunday, February 13, 2005 4:09 PM
>>> Subject: Re: About improvement of DERBY-134
>>>
>>>
>>>> Sorry.
>>>> Mistaken.
>>>>
>>>> LOOKAHEAD()....
>>>>
>>>> /*
>>>>
>>>> Tomohito Nakayama
>>>> tomoihto@rose.zero.ad.jp
>>>> tomonaka@basil.ocn.ne.jp
>>>>
>>>> Naka
>>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>>
>>>> */
>>>> ----- Original Message ----- From: "TomohitoNakayama"
>>>> <tomonaka@basil.ocn.ne.jp>
>>>> To: "Derby Development" <derby-dev@db.apache.org>
>>>> Sent: Sunday, February 13, 2005 3:42 PM
>>>> Subject: Re: About improvement of DERBY-134
>>>>
>>>>
>>>>> Hello.
>>>>>
>>>>> Thank's for your reviewing.
>>>>> Grammer ambiguity is very critical problem ....
>>>>>
>>>>> I will try to put LOOKUP() and consider about testing..
>>>>>
>>>>> #World is not simple as I wish to be.....
>>>>>
>>>>> Best regards.
>>>>>
>>>>> /*
>>>>>
>>>>> Tomohito Nakayama
>>>>> tomoihto@rose.zero.ad.jp
>>>>> tomonaka@basil.ocn.ne.jp
>>>>>
>>>>> Naka
>>>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>>>
>>>>> */
>>>>> ----- Original Message ----- From: Satheesh Bandaram
>>>>> To: Derby Development
>>>>> Sent: Saturday, February 12, 2005 4:10 AM
>>>>> Subject: Re: About improvement of DERBY-134
>>>>>
>>>>>
>>>>> I think the patch is a good start. But more work needs to be done.
>>>>> Based on a quick review, some of the items to be completed are:
>>>>> (there may be more)
>>>>>
>>>>> Grammar ambiguity. SortKey() has grammar ambiguity the way the patch
>>>>> is written. Since orderby expression and orderby column can both
>>>>> start with an identifier, this causes ambiguity. Need to rewrite or
>>>>> add lookup to avoid this.
>>>>> Current patch doesn't seem to support all expressions, Ex: select i
>>>>> from t1 order by i/2. So, needs more work.
>>>>> Add more test cases and test outputs to show changed behavior. You
>>>>> could add test cases to orderby.sql test that is already part of
>>>>> functionTests/tests/lang.
>>>>> I do encourage you to continue work on this ...
>>>>>
>>>>> Satheesh
>>>>>
>>>>> TomohitoNakayama wrote:
>>>>>
>>>>> I tried to solve DERBY-134.
>>>>> Patch is attached to this mail.
>>>>>
>>>>>
>>>>> /*
>>>>>
>>>>> Tomohito Nakayama
>>>>> tomoihto@rose.zero.ad.jp
>>>>> tomonaka@basil.ocn.ne.jp
>>>>>
>>>>> Naka
>>>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>>>
>>>>> */
>>>>> ----- Original Message ----- From: "TomohitoNakayama"
>>>>> <tomonaka@basil.ocn.ne.jp>
>>>>> To: "Derby Development" <derby-dev@db.apache.org>
>>>>> Sent: Wednesday, February 09, 2005 5:33 PM
>>>>> Subject: Re: About improvement of DERBY-134
>>>>>
>>>>>
>>>>>
>>>>> Woops.
>>>>> Mistaken.
>>>>>
>>>>>
>>>>> That's "DERBY-124 Sorted string columns are sorted in a case
>>>>> sensitive way"
>>>>>
>>>>>
>>>>>
>>>>> That's "DERBY-134 Sorted string columns are sorted in a case
>>>>> sensitive way"
>>>>>
>>>>> /*
>>>>>
>>>>> Tomohito Nakayama
>>>>> tomoihto@rose.zero.ad.jp
>>>>> tomonaka@basil.ocn.ne.jp
>>>>>
>>>>> Naka
>>>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>>>
>>>>> */
>>>>> ----- Original Message ----- From: "TomohitoNakayama"
>>>>> <tomonaka@basil.ocn.ne.jp>
>>>>> To: <derby-dev@db.apache.org>
>>>>> Sent: Wednesday, February 09, 2005 4:35 PM
>>>>> Subject: About improvement of DERBY-134
>>>>>
>>>>>
>>>>>
>>>>> Hello.
>>>>> My name is Naka.
>>>>> I'm very newbie in derby community.
>>>>>
>>>>> I'm now seeing report for derby in ASF Jira.
>>>>> And found a interesting issue.
>>>>>
>>>>> That's "DERBY-124 Sorted string columns are sorted in a case
>>>>> sensitive way"
>>>>>
>>>>> This issue seems to mean that we can't use complex item in order
>>>>> clause.
>>>>> #That title was difficult to understand a bit ....
>>>>>
>>>>> Solving this isn't useful?
>>>>> Especially when we manipulate DBMS by hand.
>>>>>
>>>>> What I think we need to do is as next:
>>>>>
>>>>> 1) Allow additiveExpression() in sortKey() in "sqlgrammer.jj". 2)
>>>>> Make OrderByColumn class to support additiveExpression.
>>>>>
>>>>> Best regards.
>>>>>
>>>>> /*
>>>>>
>>>>> Tomohito Nakayama
>>>>> tomoihto@rose.zero.ad.jp
>>>>> tomonaka@basil.ocn.ne.jp
>>>>>
>>>>> Naka
>>>>> http://www5.ocn.ne.jp/~tomohito/TopPage.html
>>>>>
>>>>> */
>>>>>
>> I have worked on Derby/Cloudscape for a few years and have even fixed
>> one or two ORDER BY bugs in the past. I have reviewed your patch. It is
>> close, but I have some problems with it.
>>
>> 1. sqlgrammar.jj. I think that creating a new method,
>> isNonReservedKeyword() to determine whether a token is a non-reserved
>> keyword or not, is a maintenance problem. Whenever we add a new
>> non-reserved keyword we must add it to the list of tokens, to
>> nonReservedKeyword(), and now to isNonReservedKeyword(). Having to add
>> it in two places is difficult enough to discover or remember. If we need
>> isNonReservedKeyword then we should find a way of combining
>> nonReservedKeyword and isNonReservedKeyword so that only one of them
>> keeps the list of non-reserved key words.
>>
>> It is not necessary for the parser to recognize 3 cases of ORDER BY sort
>> key type. A column name is just one kind of <expression>. If the parser
>> treats it as an expression we should still get the right ordering. I
>> think that it would better if the parser did so. The OrderByColumn class
>> can special case a simple column reference expression, as an
>> optimization. This considerably simplifies parsing sort keys.
>>
>> The only sort key type that has to be handled specially is that of an
>> integer constant. That specifies one of the select list columns as the
>> sort key. This case can be recognized in the parser, as is done in the
>> patch, or it can be recognized in OrderByColumn. In this alternative the
>> parser always creates OrderByColumn nodes with the sort key given by an
>> expression (a ValueNode). At bind time OrderByColumn can determine
>> whether the sort key expression is an integer constant, and if so treat
>> it as a column position.
>>
>> The two alternatives differ in the way that they treat constant integer
>> expressions like "ORDER BY 2-1". The patch orders the rows by the
>> constant 1, which is not usefull. With the patch "ORDER BY 2-1 ASC" and
>> "ORDER BY 2-1 DESC" produce the same ordering. If OrderByColumn treated
>> an integer constant sort key expression as a result column position then
>> "ORDER BY 2-1" would cause the rows to be ordered on the first result
>> column, which I think is more usefull.
>>
>> 2. OrderByColumn. I think that there is a mistake in the patch to the
>> bindOrderByColumn method of class OrderByColumn.
>>
>> The new code is
>> }else if(expression != null){
>>
>> ResultColumn col = null;
>> int i = 0;
>>
>> for(i = 0;
>> i < targetCols.size();
>> i ++){
>> col = targetCols.getOrderByColumn(i);
>> if(col != null &&
>> col.getExpression() == expression){
>> break;
>> }
>> }
>>
>> Method ResultColumnList.getOrderByColumn( int) uses 1 indexing. The
>> patch assumes 0 indexing. So the loop really should be "for( i = 1; i <=
>> targetCols.size(); i++)".
>>
>> (Java likes 0 indexing while SQL likes 1 indexing. So some parts of the
>> Derby code use 0 indexing while others use 1 indexing. The resulting
>> confusion has caught most of us at one time or another).
>>
>> The result is that when the sort key is an expression
>> OrderByColumn.pullUpOrderByColumn adds it to the end of the target list,
>> but OrderByColumn.bindOrderByColumn doesn't find it.
>> OrderByColumn.bindOrderByColumn tests whether the second last column in
>> the target list is orderable. This is usually not right. Consider the
>> following SQL:
>>
>> create table tblob( id int, b blob(1000));
>> select id,b from tblob order by abs(id);
>> select b,id from tblob order by abs(id);
>>
>> The first SELECT raises the error "ERROR X0X67: Columns of type 'BLOB'
>> may not be used in CREATE INDEX, ORDER BY, GROUP BY, UNION, INTERSECT,
>> EXCEPT or DISTINCT, because comparisons are not supported for that
>> type". The second SELECT executes properly.
>>
>> 3. Testing. I would like to see some additional tests: the failing case
>> above; ORDER BY expressions combined with ASC and DESC, to ensure that
>> the compiler handles ASC and DESC after a sort key, and comma separated
>> lists of ORDER BY expressions.
>>
>> Jack
>>
>>
>>
>>
>>
>> -- 
>> No virus found in this incoming message.
>> Checked by AVG Anti-Virus.
>> Version: 7.0.300 / Virus Database: 266.1.0 - Release Date: 2005/02/18
>>
>>
>
>
>



Mime
View raw message