db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yip Ng" <yipng...@gmail.com>
Subject Re: [jira] Updated: (DERBY-939) NullPointerException at ResultSet.close() time for simple query using UNION and INTERSECT
Date Thu, 27 Jul 2006 08:28:34 GMT
On 7/26/06, Army <qozinx@gmail.com> wrote:

> 1 - Some whitespace inconsistencies between new code and existing code.

Thanks for spotting that.  I was using 2 different IDEs with slightly
different editor settings.
I'll make the corrective action on those misaligned code that are relevent
to this patch.

> 2 - Lines longer than 80 chars

Will do the same here.

> 3 - New test file setOpPlan.sql

I was debating on whether to put it in intersect.sql or union.sql to avoid
the additional database creation during regression but then I decided to
create the new file since:

a)  This file is very specific to test set operator plans & runtime
statistics
b)  Some of the other sql files that deals with plan are very specific in
their file name, so
     I didn't want to append non-related testcases there.

So I'll stick with new file if that is ok.  I think another alternative is
to combine all the
sql files that are plan related and put them into a single sql file. This
will reduce the
number of db creations but this is really a test cleanup issue and probably
more suitable
to be addressed in another jira entry as an improvement.

Thanks for taking the time for reviewing the patch, Army.  I'll make the
above corrections
and resubmit the patch.

=============================================================

On 7/26/06, Army <qozinx@gmail.com> wrote:
>
> Yip Ng (JIRA) wrote:
> >      [ http://issues.apache.org/jira/browse/DERBY-939?page=all ]
> >
> > Yip Ng updated DERBY-939:
> > -------------------------
> >
> >     Attachment: derby939trunkdiffpatch1.txt
> >                 derby939trunkstatpatch1.txt
> >
> > Here is the patch for DERBY-939.
>
> I have reviewed the patch and the content all looks good to me.  Thanks
> for
> picking this up and for going the extra mile to actually print out a
> useful
> query plan instead of just avoiding the NPE.
>
> My only comments are very minor (and some might say annoying) ones, but I
> thought I'd make them anyways just to potentially make life easier for
> reviewers
> in the future (and to spare you the need to redo patches for picky people
> like
> me :).


<snip>

Mime
View raw message