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.ander...@oracle.com <mailto:lance.ander...@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.ander...@oracle.com <mailto:lance.ander...@oracle.com>
