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: [PATCH] Derby-127
Date Wed, 11 May 2005 16:33:30 GMT
Army wrote:

> Jack Klebanoff wrote:
>
>> I have attached a patch that fixes Jira bug Derby-127 
>> (http://issues.apache.org/jira/browse/DERBY-127).
>
>
> I reviewed this patch, applied it to a clean codeline without problem, 
> and ran the orderby.sql test that was included.  From what I can tell, 
> everything looks good here.
>
> My only minor comment is that it might be nice to add a case to the 
> orderby.sql test to make sure things work if _multiple_ columns are 
> provided in an order by clause.  Ex.
>
> ij> select c1 as x, c2 as y from bug2769 group by bug2769.c1, 
> bug2769.c2 order by c1, c2;
>
> ij> select c1 as x, c2 as y from bug2769 group by bug2769.c1, 
> bug2769.c2 order by c1, y;
>
> I tried these and they both work fine--no problems there.  But it was 
> something I was wondering while I was reviewing, so it might be nice 
> to include it in the test...*shrug*
>
> In any event, the patch gets my +1,
> Army
>
>
I agree with Army's suggestion for an extra test. Should I wait for my 
original patch to be committed and submit the new test in a new patch, 
or should I revise and re-submit my patch? Since Army has shown that the 
patch does handle his test case I suggest committing my current patch 
first. This will get things moving. Adding a few cases to an existing 
test results in a small, easily reviewed patch. If I revise the 
submitted patch it will be more difficult to review the submission.

Jack Klebanoff

Mime
View raw message