db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jack Klebanoff <klebanoff-de...@sbcglobal.net>
Subject Re: About improvement of DERBY-134
Date Sat, 19 Mar 2005 00:04:54 GMT
The derbyall test suite found a problem. The lang/wisconsin.sql test
failed. The problem output was:

ij> -- Values clause is a single-row result set, so should not cause
optimizer
-- to require sort.
get cursor c as
'select * from TENKTUP1, (values 1) as t(x)
where TENKTUP1.unique1 = t.x
order by TENKTUP1.unique1, t.x';
ERROR 42X10: 'T' is not an exposed table name in the scope in which it
appears.

This error is incorrect.

There must be a problem in the way that the patch binds the ORDER BY
expressions. I don't have time to look more deeply into it now.

You should probably run at least the derbylang test suite before
submitting a patch for ORDER BY.

To do this, change into an empty directory and run
java org.apache.derbyTesting.functionTests.harness.RunSuite derbylang
The derbylang suite takes about 90 minutes on my laptop. The derbyall
suite takes 5 or 6 hours.

In order to run just the wisconsin.sql test change into an empty
directory and run
java org.apache.derbyTesting.functionTests.harness.RunTest
lang/wisconsin.sql

Jack Klebanoff

TomohitoNakayama wrote:

> 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
>>>>>>
>>>>>>
>>>>>>
>>


Mime
View raw message