> 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 <[EMAIL PROTECTED]> wrote:
<snip>
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>
