[ 
https://issues.apache.org/jira/browse/DERBY-4166?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12735148#action_12735148
 ] 

Kathey Marsden commented on DERBY-4166:
---------------------------------------

Thanks Lily for the patch. 
- In schema.sql there is a small space/tab issue at:
                     attach_id bigint generated always as identity (start with 
1, increment by 1),

- in DbTasks.deleteMailByThread there is a count_att which I think is not used.
- In insertMail I think the code to insert the attachements should be included 
in the loop to insert the mail in the INBOX and should not require a select 
from INBOX after the .  In general we want.
                int num = Rn.nextInt(10 - 1);
                        for (int i = 0; i < num; i++) {
                             //insert mail to INBOX 
                             ... code looks fine
                              // get the id. with insertFirst.getGeneratedKeys()
                                .. need to add                    
                             // insert attachments  for approximately 1 in 10 
mails
                             if  (i == Rn.nextInt(10 -1)) {
                              // insert from 1 to 5 attachments
                              num_attatch - Rn.nextInt(5-1);
                              for (int j = 0; j < num_attach;j++) {
                                    // insert the attachment  using the id you 
got from getGeneratedKeys()
                                }
                               }


> improvements to the mailjdbc test
> ---------------------------------
>
>                 Key: DERBY-4166
>                 URL: https://issues.apache.org/jira/browse/DERBY-4166
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.6.0.0
>            Reporter: Kathey Marsden
>            Priority: Minor
>         Attachments: DERBY-4166-databasesize.diff, Derby-4166-samedb.diff, 
> DERBY-4166-schemachange.diff, Derby-4166.diff
>
>
> When recently working with the mailjdbc system test 
> org.apache.derbyTesting.system.mailjdbc on DERBY-4152 I noticed some 
> potential improvements that might be good for the test.  We should probably 
> hold off on these improvements however until the root cause of DERBY-4152 is 
> established, however, so we don't muddy the waters with that issue by 
> changing the test.
> 1) DbTasks.moveToFolders may throw an IllegalArgumentException.
>   There is a line:  message_id = Rn.nextInt(count - 1);
>   if count is 1 the argument to nextInt() might be 0 which is not allowed.  I 
> hit this once but lost the stack trace, but it is apparent that when there is 
> only one row in the table this can occur.
>  
> 2) Allow/implement multiple attachments per message and cleanup 
> DbTasks.insertMail() logic.
>    - Remove the attach_id column from INBOX to allow multiple attachments.
>    -Make the attachment insert part of the message for loop in insertMail.
>    Use getGeneratedKeys() to get the id of the inserted message.
>    When attachments are inserted, insert (1-4) attachments and give them a 
> corresponding attach_id from 1-4.
> This will allow for removal of the select statements used to determine id and 
> attach_id.  I'll file another issue for these improvements if folks agree 
> that they are sensible.
> A detailed description of the current implementation of insertMail is 
> described at 
> https://issues.apache.org/jira/secure/attachment/12405685/insertMailSummary.txt
> 3) DbTasks.databaseSize calculation is wrong. It doesn't match du -sk. The 
> method does not recurse into subdirectories and includes the length() on 
> directory files which is undefined accourding to the file.length() javadoc.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to