Since you are working on an enhancement to Derby, you need to sign a
CLA with Apache.
Jean, is this the right CLA?
http://www.apache.org/~tetsuya/db/agreement.html
Satheesh
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
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "Satheesh Bandaram"
<[EMAIL PROTECTED]>
To: "Derby Development" <[email protected]>
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
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "Jack Klebanoff"
<[EMAIL PROTECTED]>
To: "Derby Development" <[email protected]>
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
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "TomohitoNakayama"
<[EMAIL PROTECTED]>
To: "Derby Development" <[email protected]>
Sent: Sunday, February 13, 2005 4:09 PM
Subject: Re: About improvement of DERBY-134
Sorry.
Mistaken.
LOOKAHEAD()....
/*
Tomohito Nakayama
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "TomohitoNakayama"
<[EMAIL PROTECTED]>
To: "Derby Development" <[email protected]>
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
[EMAIL PROTECTED]
[EMAIL PROTECTED]
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
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "TomohitoNakayama"
<[EMAIL PROTECTED]>
To: "Derby Development" <[email protected]>
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
[EMAIL PROTECTED]
[EMAIL PROTECTED]
Naka
http://www5.ocn.ne.jp/~tomohito/TopPage.html
*/
----- Original Message ----- From: "TomohitoNakayama"
<[EMAIL PROTECTED]>
To: <[email protected]>
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
[EMAIL PROTECTED]
[EMAIL PROTECTED]
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
--
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
No virus found in this outgoing message.
Checked by AVG Anti-Virus.
Version: 7.0.308 / Virus Database: 266.7.2 - Release Date: 2005/03/11
|
