I have committed this patch. If you would like to submit test updates later, that would be fine.
Satheesh Jack Klebanoff wrote: > 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 > > >
