db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "TomohitoNakayama" <tomon...@basil.ocn.ne.jp>
Subject Re: About improvement of DERBY-134
Date Fri, 18 Mar 2005 14:33:14 GMT
Hello.

Thank for your checking.
I have solved the 2 problems.
Attached file is new patch.

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: Tuesday, March 15, 2005 10:51 AM
Subject: Re: About improvement of DERBY-134


> The new patch looks much better. However, I found two problems, one
> serious and the other minor.
>
> The serious problem is that INTERSECT no longer works. The
> lang/intersect.sql test (part of the derbylang suite) fails. The problem
> is in the
> org.apache.derby.impl.sql.compile.IntersectOrExceptNode.pushOrderingDown
> method. It attempts to create OrderByColumns by calling
> nf.getNode( C_NodeTypes.ORDER_BY_COLUMN,
> ReuseFactory.getInteger( intermediateOrderByColumns[i] + 1),
> cm)
> This used to work. Now OrderByColumn.init throws a ClassCastException
> because it expects to be passed a ValueNode, not an Integer.
>
> IntersectOrExceptNode.pushOrderingDown has to be changed to pass a
> ValueNode. I think that
> nf.getNode( C_NodeTypes.ORDER_BY_COLUMN,
> nf.getNode( C_NodeTypes.INT_CONSTANT_NODE,
> ReuseFactory.getInteger( intermediateOrderByColumns[i] + 1),
> cm),
> cm)
> works.
>
> The minor problem is that the javadoc for OrderByColumn.init( Object
> expression) documents a "dummy" parameter that no longer exists.
>
> Jack Klebanoff
>
> TomohitoNakayama wrote:
>
>> Hello.
>>
>> I have finished coding and testing in orderby.sql.
>> I'm not sure test is enough.
>>
>> Would you please review it ?
>>
>> 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"
>> <satheesh@sourcery.org>
>> To: "Derby Development" <derby-dev@db.apache.org>
>> Sent: Saturday, March 12, 2005 6:59 AM
>> Subject: Re: About improvement of DERBY-134
>>
>>
>>> 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.308 / Virus Database: 266.7.2 - Release Date: 2005/03/11
> 

Mime
View raw message