Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Deven, This was pushed a while ago you should have seen the putback on the alias See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a8978a5bb6e Best Lance On Jan 10, 2013, at 12:49 AM, Deven You wrote: Hi Lance, Is there any update for this issue. If you have anything I can do, please let me know. Thanks a lot! On 11/24/2012 12:45 AM, Lance Andersen - Oracle wrote: It is on my list. to update the javadocs I need a ccc which I have not done yet and is needed as part of this change On Nov 23, 2012, at 3:07 AM, Deven You wrote: Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
The JCK and RowSet TCK will have base tests to validate. I have a separate set of unit tests for JDBC which includes more detailed tests than what you had suggested as well as end to end tests. Best Lance On Jan 10, 2013, at 8:24 AM, Deven You wrote: Hi Lance, I am glad to see the patch for implementing these methods are committed. Do you have a plan to add the test cases[1] I created too? Thanks a lot! [1]http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ On 01/10/2013 08:31 PM, Lance Andersen - Oracle wrote: Deven, This was pushed a while ago you should have seen the putback on the alias See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a8978a5bb6e Best Lance On Jan 10, 2013, at 12:49 AM, Deven You wrote: Hi Lance, Is there any update for this issue. If you have anything I can do, please let me know. Thanks a lot! On 11/24/2012 12:45 AM, Lance Andersen - Oracle wrote: It is on my list. to update the javadocs I need a ccc which I have not done yet and is needed as part of this change On Nov 23, 2012, at 3:07 AM, Deven You wrote: Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance 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 http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance 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 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Hi Lance, Is there any update for this issue. If you have anything I can do, please let me know. Thanks a lot! On 11/24/2012 12:45 AM, Lance Andersen - Oracle wrote: It is on my list. to update the javadocs I need a ccc which I have not done yet and is needed as part of this change On Nov 23, 2012, at 3:07 AM, Deven You wrote: Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance 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
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
It is on my list. to update the javadocs I need a ccc which I have not done yet and is needed as part of this change On Nov 23, 2012, at 3:07 AM, Deven You wrote: Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan.
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
On 21/09/2012 04:21, Deven You wrote: 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? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan.
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
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.gifLance 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.gifLance 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
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
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.gifLance 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
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
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 Thanks a lot. -- Best Regards, Deven Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com -- Best Regards, Deven Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
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 Thanks a lot. -- Best Regards, Deven
Re: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
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 Thanks a lot. -- Best Regards, Deven Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
