javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream

[Deven You]

Hi Lance,

I am very busy with other work so I can't work with the 
SerialBlob/SerialClob item for long time. I am very happy to refine the 
current test case and create new tests for SerialClob.

I have create a new webre[1] for this task, please review it.

[1] http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ 
<http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/>

PS: If the isFree is not transient, I want to know how we add this field 
to the javadoc serialized form?

Thanks a lot!

On 07/06/2012 04:46 AM, Lance Andersen - Oracle wrote:
> Hi Deven,
>
> We had a holiday here on Weds,  so I am still catching up.
>
> First, thank you for taking the time to propose a patch.
>
>  Here are a few comments based on the changes I have made in my 
> workspace and  comparing to your proposed changes
>
> - The same issue applies to SerialClob
> - instead of duplicating all of the code to check if free has been 
> called in each method, I created a private method which does the 
> validation
> - I do not see a reason to make the flag isFree transient, if the 
> object has been freed, it has been freed
> - free() needs to call blob.free if the blob object is not null prior 
> to nulling out the object
> - Your 2nd if statement in getBinaryStream() duplicates part of the 
> check in the 1st if statement
> - All of the public methods need to have their javadocs indicate that 
> if called after free has been called, a SerialException will be 
> thrown.  additional calls to free is a no-op
> - The test case is is a good start.    I would change this a bit though:
>   + create separate test classes for free and getbinarystream
>   + each individual test  be a method
>   + If you catch the expected Exception, print that the test has passed
>   + Each test needs a comment as to what it is trying to validate 
> (just a simple comment is fine)
>   + return 0 if the test passed, 1 if it fails  and then print the 
> number of failed tests at the end after running through all of them
>
>
> As I need to change the javadocs, I have create  a ccc request which I 
> will do as part of the JDBC 4.2 work and will put the change back at 
> that time.
>
> If you would like to change your test and then create similar tests 
> for SerialClob I will add those as part of the put-back.
>
> Again, thank you for your input and time.
>
> Best
> Lance
>
>
>
>
> On Jul 5, 2012, at 2:26 AM, Deven You wrote:
>
>> Hi Lance,
>>
>> Did you review the patch and compare it to yours. I just have some 
>> more words for the patch.
>>
>> 1. I think the current spec for free() is not clear, how about add 
>> below comments:
>>
>>     After free has been called, any attempt to invoke a method other 
>> than free will result in a SerialException being thrown.
>>
>> 2. getBinaryStream(long pos,long length)
>>
>>     add a javadoc:
>>     * @throws SerialException if this SerialBlob already be freed.
>>
>>     add throws SerialException from this method
>>
>> Any suggestions?
>>
>> Thanks a lot!
>> On 07/02/2012 06:25 PM, Lance Andersen - Oracle wrote:
>>> Hi Deven,
>>>
>>> Thanks for the email and the proposed patch.  I will look at this 
>>> later today or tomorrow.  I actually have made these changes in my 
>>> workspace for JDK 8 but will compare your changes to mine.
>>>
>>> Best
>>> Lance
>>> On Jul 2, 2012, at 5:04 AM, Deven You wrote:
>>>
>>>> Hi All,
>>>> Could anyone notice this problem?
>>>>
>>>> Thanks a lot!
>>>> On 06/25/2012 04:18 PM, Deven You wrote:
>>>>> Hi All,
>>>>>
>>>>> First of all, if the jdbc problem has a better mailing list to 
>>>>> post please tell me.
>>>>>
>>>>> I find that javax.sql.rowset.serial.SerialBlob is not fully 
>>>>> implemented in OpenJDK 8. Methods
>>>>>
>>>>>    public InputStream getBinaryStream(long pos,long length) throws 
>>>>> SQLException
>>>>>    public void free() throws SQLException
>>>>>
>>>>> only throw UnsupportedOperationException.
>>>>>
>>>>> I have made a patch[1] to implement these 2 methods. Could anyone 
>>>>> take a look to review it.
>>>>>
>>>>> BTW: I think the spec for SerialBlob is not very clear like it 
>>>>> doesn't mention if all method rather than free() need throw any 
>>>>> exception after free() is invoked. However that behavior seems 
>>>>> reasonable.
>>>>>
>>>>> [1] http://cr.openjdk.java.net/~littlee/OJDK-576/webrev 
>>>>> <http://cr.openjdk.java.net/%7Elittlee/OJDK-576/webrev>
>>>>>
>>>>> Thanks a lot.
>>>>>
>>>>
>>>>
>>>> -- 
>>>> Best Regards,
>>>>
>>>> Deven
>>>>
>>>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
>>> Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>>>
>>
>>
>> -- 
>> Best Regards,
>>
>> Deven
>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
> Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>