Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
On 15/01/2013 16:48, Lance Andersen - Oracle wrote: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. It would be good to get the ClassCastException specified when you get time. I looked at the latest webrev and it seems okay to me. SQLInputImpl.getNextAttribute would be bit better if it only incremented idx after retrieving an attribute but that is existing code. Minor intent issue in SQLOutputImpl. writeNClob. -Alan.
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
On Jan 19, 2013, at 9:43 AM, Alan Bateman wrote: On 15/01/2013 16:48, Lance Andersen - Oracle wrote: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. It would be good to get the ClassCastException specified when you get time. Yes will do once the ccc is approved on my list I looked at the latest webrev and it seems okay to me. SQLInputImpl.getNextAttribute would be bit better if it only incremented idx after retrieving an attribute but that is existing code. Minor intent issue in SQLOutputImpl. writeNClob. fixed the spacing. going to leave idx alone for now as but will look at this for the future Best Lance -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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Looks great! Little nit: SQLOutputImpl.java line 579 could be dropped. -Ulf Am 15.01.2013 17:48, schrieb Lance Andersen - Oracle: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Thank you Ulf. I deleted the extra line on 579 Best Lance On Jan 15, 2013, at 3:45 PM, Ulf Zibis wrote: Looks great! Little nit: SQLOutputImpl.java line 579 could be dropped. -Ulf Am 15.01.2013 17:48, schrieb Lance Andersen - Oracle: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -Alan
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Yes sorry if that was not clear but that is my plan with the ccc and javadoc update I will get the JBS entry and ccc submitted later today and make a few of the minor suggestions from ulf and push out the revised webrev later today or Tuesday Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Sent from my iPad On Jan 14, 2013, at 5:11 AM, Alan Bateman alan.bate...@oracle.com wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -Alan
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Am 12.01.2013 22:56, schrieb Lance Andersen - Oracle:
Hi
This is a review request for 8006139 which adds missing methods to
SQLInput/Output
The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/
Hi,
are you paid by code lines ;-)
Additionally you have left out one opportunity
... or is it new code style, having 2 wrong indented closing braces in one line?
I would code:
811 public String readNString() throws SQLException {
812 String attrib = (String)getNextAttribute();
813 lastValueWasNull =attrib == null;
814return attrib;
815 }
Additionally I'm wondering, whether getNextAttribute() could be generified.
-Ulf
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Oops, missing spaces by Thunderbird failure, correction:
811 public String readNString() throws SQLException {
812 String attrib = (String)getNextAttribute();
813 lastValueWasNull = attrib == null;
814 return attrib;
815 }
Additionally I'm wondering, whether getNextAttribute() could be generified.
... and again more general:
xxx private T T readNext() throws SQLException {
xxx T attrib = (T)getNextAttribute();
xxx lastValueWasNull = attrib == null;
xxx return attrib;
xxx }
811 public String readNString() throws SQLException {
812 return readNext();
813 }
-Ulf
Am 13.01.2013 11:56, schrieb Ulf Zibis:
Am 12.01.2013 22:56, schrieb Lance Andersen - Oracle:
Hi
This is a review request for 8006139 which adds missing methods to
SQLInput/Output
The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/
Hi,
are you paid by code lines ;-)
Additionally you have left out one opportunity
... or is it new code style, having 2 wrong indented closing braces in one line?
I would code:
811 public String readNString() throws SQLException {
812 String attrib = (String)getNextAttribute();
813 lastValueWasNull =attrib == null;
814return attrib;
815 }
Additionally I'm wondering, whether getNextAttribute() could be generified.
-Ulf
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Sent from my iPad
On Jan 13, 2013, at 5:56 AM, Ulf Zibis ulf.zi...@cosoco.de wrote:
Am 12.01.2013 22:56, schrieb Lance Andersen - Oracle:
Hi
This is a review request for 8006139 which adds missing methods to
SQLInput/Output
The webrev can be found at
http://cr.openjdk.java.net/~lancea/8006139/webrev.00/
Hi,
are you paid by code lines ;-)
Additionally you have left out one opportunity
... or is it new code style, having 2 wrong indented closing braces in one
line?
Yes I should have caught that and will fix
I would code:
811 public String readNString() throws SQLException {
812 String attrib = (String)getNextAttribute();
813 lastValueWasNull =attrib == null;
814return attrib;
815 }
Additionally I'm wondering, whether getNextAttribute() could be generified.
Yes it probably could however I do no want to do this as part of this change
-Ulf
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Am 12.01.2013 22:56, schrieb Lance Andersen - Oracle: Hi This is a review request for 8006139 which adds missing methods to SQLInput/Output The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/ All @since tags are missing in these classes :-( -Ulf
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Yes I just noticed this The code was written before my time I will add them though Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Sent from my iPad On Jan 13, 2013, at 7:28 AM, Ulf Zibis ulf.zi...@cosoco.de wrote: Am 12.01.2013 22:56, schrieb Lance Andersen - Oracle: Hi This is a review request for 8006139 which adds missing methods to SQLInput/Output The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/ All @since tags are missing in these classes :-( -Ulf
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
On 01/13/2013 01:24 PM, Lance @ Oracle wrote:
On Jan 13, 2013, at 5:56 AM, Ulf Zibis ulf.zi...@cosoco.de wrote:
Am 12.01.2013 22:56, schrieb Lance Andersen - Oracle:
Hi
This is a review request for 8006139 which adds missing methods to
SQLInput/Output
The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/
Hi,
are you paid by code lines ;-)
Additionally you have left out one opportunity
... or is it new code style, having 2 wrong indented closing braces in one line?
Yes I should have caught that and will fix
I would code:
811 public String readNString() throws SQLException {
812 String attrib = (String)getNextAttribute();
813 lastValueWasNull =attrib == null;
814return attrib;
815 }
Additionally I'm wondering, whether getNextAttribute() could be generified.
Yes it probably could however I do no want to do this as part of this change
I disagree, it should not be generified, getNextAttribute() should
return Object and be casted at callsite,
it will more clear if a ClassCastException occurs.
Relying on inference to ask the compiler to insert the cast is dubious,
-Ulf
Rémi
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Am 13.01.2013 13:33, schrieb Remi Forax:
Additionally I'm wondering, whether getNextAttribute() could be generified.
Yes it probably could however I do no want to do this as part of this change
I disagree, it should not be generified, getNextAttribute() should return Object and be casted at
callsite,
it will more clear if a ClassCastException occurs.
Relying on inference to ask the compiler to insert the cast is dubious,
I must admit, I do not understand.
What would be wrong with that? :
xxx private T T readNext() throws SQLException {
xxx T attrib = (T)getNextAttribute();
xxx lastValueWasNull = attrib == null;
xxx return attrib;
xxx }
811 public String readNString() throws SQLException {
812 return readNext();
813 }
... so why not generifying getNextAttribute() directly?
-Ulf
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
On 12/01/2013 21:56, Lance Andersen - Oracle wrote: Hi This is a review request for 8006139 which adds missing methods to SQLInput/Output The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/ Lance - I think this will require a spec clarification so that it's clear that ClassCastException can be thrown by these methods. Since this doesn't appear to have been specifically previously then I think you might need to check the current behavior of existing drivers (as they will need to be changed if they don't throw CCE already). -Alan
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
I will enter a jbs entry on Monday for this Have a good weekend Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Sent from my iPhone On Jan 13, 2013, at 8:37 AM, Alan Bateman alan.bate...@oracle.com wrote: On 12/01/2013 21:56, Lance Andersen - Oracle wrote: Hi This is a review request for 8006139 which adds missing methods to SQLInput/Output The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/ Lance - I think this will require a spec clarification so that it's clear that ClassCastException can be thrown by these methods. Since this doesn't appear to have been specifically previously then I think you might need to check the current behavior of existing drivers (as they will need to be changed if they don't throw CCE already). -Alan
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
On 13/01/2013 15:07, Lance Andersen wrote: I will enter a jbs entry on Monday for this Assuming CCE is long standing behavior then it would be good to update the spec of all the other read* methods too. Alternatively a statement in the class description to cover it. One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. -Alan
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Am 13.01.2013 21:08, schrieb Alan Bateman: Assuming CCE is long standing behavior then it would be good to update the spec of all the other read* methods too. Alternatively a statement in the class description to cover it. Only using a generified version of readObject(), a CCE would be less surprisingly to the user. The other methods then could be deprecated. One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. Maybe we could add a move-back method, so the stream-like behaviour would be more clear. Was this API ever thread-safe? Does it make any sense to use it multi-threaded? -Ulf
review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Hi This is a review request for 8006139 which adds missing methods to SQLInput/Output The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/ best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
