You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by Shreyas Kaushik <Sh...@Sun.COM> on 2005/03/22 09:03:25 UTC
[PATCH] Derby-174
Find the patch for this attached. I will send the test patch in a
seperate mail.
~ Shreyas
Re: [PATCH] Derby-174
Posted by Shreyas Kaushik <Sh...@Sun.COM>.
Thanks Satheesh.
Please find the patch attached, after adding this test to the
jdbcapi.runall file.
~ Shreyas
Satheesh Bandaram wrote:
>I have submitted this patch. Thanks for adding new test case to test
>both. But the new test is not part of any test suite, so it may not get
>run even though it is there. Can you add the test case into 'jdbcapi'
>or 'derbylang' test suites?
>
>Sending java\engine\org\apache\derby\iapi\types\SQLBlob.java
>Sending java\engine\org\apache\derby\iapi\types\SQLTimestamp.java
>Adding
>java\testing\org\apache\derbyTesting\functionTests\master\prepStmtNull.out
>Adding
>java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\prepStmtNull.java
>Transmitting file data ....
>Committed revision 159445.
>
>Shreyas Kaushik wrote:
>
>
>
>>Sateesh,
>>
>>Did you have a chance to look into this?
>>
>>~ Shreyas
>>
>>Shreyas Kaushik wrote:
>>
>>
>>
>>>Attached is the latest patch with the changes.
>>>
>>>thanks
>>>Shreyas
>>>
>>>Satheesh Bandaram wrote:
>>>
>>>
>>>
>>>>I will submit your patches after your changes.
>>>>
>>>>Satheesh
>>>>
>>>>Shreyas Kaushik wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>I guess this is an IDE trick , I'll change this.
>>>>>
>>>>>~ Shreyas
>>>>>
>>>>>Army wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>><Sh...@sun.com> wrote
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>-import java.sql.Date;
>>>>>>>-import java.sql.Time;
>>>>>>>-import java.sql.Timestamp;
>>>>>>>-import java.sql.Types;
>>>>>>>-import java.sql.ResultSet;
>>>>>>>-import java.sql.SQLException;
>>>>>>>+import java.sql.*;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>I apologize if this is overly picky, but isn't inclusion of a package
>>>>>>via the "*" wildcard generally considered an unfavorable programming
>>>>>>practice? I think it's fine to do so when writing tests, but as far
>>>>>>as I know, the Derby codeline generally avoids using wildcard
>>>>>>imports. And even if there are existing places where such imports
>>>>>>are used, it seems to me that _replacing_ existing imports with the
>>>>>>wildcard import is probably not a habit to encourage.
>>>>>>
>>>>>>Even if functionally speaking there's no difference, to do this is to
>>>>>>introduce a style of imports that's doesn't agree with the rest of
>>>>>>the codeline--is that something to avoid?
>>>>>>
>>>>>>'Course, maybe that's just unfounded paranoia on my part...
>>>>>>
>>>>>>*shrug*
>>>>>>Army
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>------------------------------------------------------------------------
>>>
>>>Index: java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>>>===================================================================
>>>--- java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>>>(revision 158017)
>>>+++ java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>>>(working copy)
>>>@@ -45,12 +45,15 @@
>>>import org.apache.derby.iapi.types.SQLDouble;
>>>import org.apache.derby.iapi.types.SQLTime;
>>>
>>>+
>>>+
>>>import java.sql.Date;
>>>import java.sql.Time;
>>>import java.sql.Timestamp;
>>>import java.sql.Types;
>>>import java.sql.ResultSet;
>>>import java.sql.SQLException;
>>>+import java.sql.PreparedStatement;
>>>
>>>import java.util.Calendar;
>>>import java.util.GregorianCalendar;
>>>@@ -65,7 +68,7 @@
>>>/**
>>> * This contains an instance of a SQL Timestamp object.
>>> * <p>
>>>- * SQLTimestamp is stored in 3 ints - an encoded date, an encoded
>>>time and + * SQLTimestamp is stored in 3 ints - an encoded date, an
>>>encoded time and
>>> * nanoseconds
>>> * encodedDate = 0 indicates a null WSCTimestamp
>>> *
>>>@@ -989,5 +992,12 @@
>>> currentCal.setTime(value);
>>> return SQLTime.computeEncodedTime(currentCal);
>>> }
>>>+
>>>+ + public void setInto(PreparedStatement ps, int position)
>>>throws SQLException, StandardException {
>>>+
>>>+ ps.setTimestamp(position, getTimestamp((Calendar)
>>>null));
>>>+ }
>>>}
>>>
>>>+
>>>
>>>
>>>
>>>
>>
>>
>>
>
>
>
Re: [PATCH] Derby-174
Posted by Satheesh Bandaram <sa...@Sourcery.Org>.
I have submitted this patch. Thanks for adding new test case to test
both. But the new test is not part of any test suite, so it may not get
run even though it is there. Can you add the test case into 'jdbcapi'
or 'derbylang' test suites?
Sending java\engine\org\apache\derby\iapi\types\SQLBlob.java
Sending java\engine\org\apache\derby\iapi\types\SQLTimestamp.java
Adding
java\testing\org\apache\derbyTesting\functionTests\master\prepStmtNull.out
Adding
java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\prepStmtNull.java
Transmitting file data ....
Committed revision 159445.
Shreyas Kaushik wrote:
> Sateesh,
>
> Did you have a chance to look into this?
>
> ~ Shreyas
>
> Shreyas Kaushik wrote:
>
>> Attached is the latest patch with the changes.
>>
>> thanks
>> Shreyas
>>
>> Satheesh Bandaram wrote:
>>
>>> I will submit your patches after your changes.
>>>
>>> Satheesh
>>>
>>> Shreyas Kaushik wrote:
>>>
>>>
>>>
>>>> I guess this is an IDE trick , I'll change this.
>>>>
>>>> ~ Shreyas
>>>>
>>>> Army wrote:
>>>>
>>>>
>>>>
>>>>> <Sh...@sun.com> wrote
>>>>>
>>>>>
>>>>>
>>>>>> -import java.sql.Date;
>>>>>> -import java.sql.Time;
>>>>>> -import java.sql.Timestamp;
>>>>>> -import java.sql.Types;
>>>>>> -import java.sql.ResultSet;
>>>>>> -import java.sql.SQLException;
>>>>>> +import java.sql.*;
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> I apologize if this is overly picky, but isn't inclusion of a package
>>>>> via the "*" wildcard generally considered an unfavorable programming
>>>>> practice? I think it's fine to do so when writing tests, but as far
>>>>> as I know, the Derby codeline generally avoids using wildcard
>>>>> imports. And even if there are existing places where such imports
>>>>> are used, it seems to me that _replacing_ existing imports with the
>>>>> wildcard import is probably not a habit to encourage.
>>>>>
>>>>> Even if functionally speaking there's no difference, to do this is to
>>>>> introduce a style of imports that's doesn't agree with the rest of
>>>>> the codeline--is that something to avoid?
>>>>>
>>>>> 'Course, maybe that's just unfounded paranoia on my part...
>>>>>
>>>>> *shrug*
>>>>> Army
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>>
>> ------------------------------------------------------------------------
>>
>> Index: java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>> ===================================================================
>> --- java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>> (revision 158017)
>> +++ java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>> (working copy)
>> @@ -45,12 +45,15 @@
>> import org.apache.derby.iapi.types.SQLDouble;
>> import org.apache.derby.iapi.types.SQLTime;
>>
>> +
>> +
>> import java.sql.Date;
>> import java.sql.Time;
>> import java.sql.Timestamp;
>> import java.sql.Types;
>> import java.sql.ResultSet;
>> import java.sql.SQLException;
>> +import java.sql.PreparedStatement;
>>
>> import java.util.Calendar;
>> import java.util.GregorianCalendar;
>> @@ -65,7 +68,7 @@
>> /**
>> * This contains an instance of a SQL Timestamp object.
>> * <p>
>> - * SQLTimestamp is stored in 3 ints - an encoded date, an encoded
>> time and + * SQLTimestamp is stored in 3 ints - an encoded date, an
>> encoded time and
>> * nanoseconds
>> * encodedDate = 0 indicates a null WSCTimestamp
>> *
>> @@ -989,5 +992,12 @@
>> currentCal.setTime(value);
>> return SQLTime.computeEncodedTime(currentCal);
>> }
>> +
>> + + public void setInto(PreparedStatement ps, int position)
>> throws SQLException, StandardException {
>> +
>> + ps.setTimestamp(position, getTimestamp((Calendar)
>> null));
>> + }
>> }
>>
>> +
>>
>>
>
>
>
Re: [PATCH] Derby-174
Posted by Shreyas Kaushik <Sh...@Sun.COM>.
Sateesh,
Did you have a chance to look into this?
~ Shreyas
Shreyas Kaushik wrote:
> Attached is the latest patch with the changes.
>
> thanks
> Shreyas
>
> Satheesh Bandaram wrote:
>
>> I will submit your patches after your changes.
>>
>> Satheesh
>>
>> Shreyas Kaushik wrote:
>>
>>
>>
>>> I guess this is an IDE trick , I'll change this.
>>>
>>> ~ Shreyas
>>>
>>> Army wrote:
>>>
>>>
>>>
>>>> <Sh...@sun.com> wrote
>>>>
>>>>
>>>>
>>>>> -import java.sql.Date;
>>>>> -import java.sql.Time;
>>>>> -import java.sql.Timestamp;
>>>>> -import java.sql.Types;
>>>>> -import java.sql.ResultSet;
>>>>> -import java.sql.SQLException;
>>>>> +import java.sql.*;
>>>>>
>>>>
>>>>
>>>> I apologize if this is overly picky, but isn't inclusion of a package
>>>> via the "*" wildcard generally considered an unfavorable programming
>>>> practice? I think it's fine to do so when writing tests, but as far
>>>> as I know, the Derby codeline generally avoids using wildcard
>>>> imports. And even if there are existing places where such imports
>>>> are used, it seems to me that _replacing_ existing imports with the
>>>> wildcard import is probably not a habit to encourage.
>>>>
>>>> Even if functionally speaking there's no difference, to do this is to
>>>> introduce a style of imports that's doesn't agree with the rest of
>>>> the codeline--is that something to avoid?
>>>>
>>>> 'Course, maybe that's just unfounded paranoia on my part...
>>>>
>>>> *shrug*
>>>> Army
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
>>
>------------------------------------------------------------------------
>
>Index: java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>===================================================================
>--- java/engine/org/apache/derby/iapi/types/SQLTimestamp.java (revision 158017)
>+++ java/engine/org/apache/derby/iapi/types/SQLTimestamp.java (working copy)
>@@ -45,12 +45,15 @@
> import org.apache.derby.iapi.types.SQLDouble;
> import org.apache.derby.iapi.types.SQLTime;
>
>+
>+
> import java.sql.Date;
> import java.sql.Time;
> import java.sql.Timestamp;
> import java.sql.Types;
> import java.sql.ResultSet;
> import java.sql.SQLException;
>+import java.sql.PreparedStatement;
>
> import java.util.Calendar;
> import java.util.GregorianCalendar;
>@@ -65,7 +68,7 @@
> /**
> * This contains an instance of a SQL Timestamp object.
> * <p>
>- * SQLTimestamp is stored in 3 ints - an encoded date, an encoded time and
>+ * SQLTimestamp is stored in 3 ints - an encoded date, an encoded time and
> * nanoseconds
> * encodedDate = 0 indicates a null WSCTimestamp
> *
>@@ -989,5 +992,12 @@
> currentCal.setTime(value);
> return SQLTime.computeEncodedTime(currentCal);
> }
>+
>+
>+ public void setInto(PreparedStatement ps, int position) throws SQLException, StandardException {
>+
>+ ps.setTimestamp(position, getTimestamp((Calendar) null));
>+ }
> }
>
>+
>
>
Re: [PATCH] Derby-174
Posted by Shreyas Kaushik <Sh...@Sun.COM>.
Attached is the latest patch with the changes.
thanks
Shreyas
Satheesh Bandaram wrote:
>I will submit your patches after your changes.
>
>Satheesh
>
>Shreyas Kaushik wrote:
>
>
>
>>I guess this is an IDE trick , I'll change this.
>>
>>~ Shreyas
>>
>>Army wrote:
>>
>>
>>
>>><Sh...@sun.com> wrote
>>>
>>>
>>>
>>>>-import java.sql.Date;
>>>>-import java.sql.Time;
>>>>-import java.sql.Timestamp;
>>>>-import java.sql.Types;
>>>>-import java.sql.ResultSet;
>>>>-import java.sql.SQLException;
>>>>+import java.sql.*;
>>>>
>>>>
>>>
>>>I apologize if this is overly picky, but isn't inclusion of a package
>>>via the "*" wildcard generally considered an unfavorable programming
>>>practice? I think it's fine to do so when writing tests, but as far
>>>as I know, the Derby codeline generally avoids using wildcard
>>>imports. And even if there are existing places where such imports
>>>are used, it seems to me that _replacing_ existing imports with the
>>>wildcard import is probably not a habit to encourage.
>>>
>>>Even if functionally speaking there's no difference, to do this is to
>>>introduce a style of imports that's doesn't agree with the rest of
>>>the codeline--is that something to avoid?
>>>
>>>'Course, maybe that's just unfounded paranoia on my part...
>>>
>>>*shrug*
>>>Army
>>>
>>>
>>>
>>
>>
>>
>
>
>
Re: [PATCH] Derby-174
Posted by Satheesh Bandaram <sa...@Sourcery.Org>.
I will submit your patches after your changes.
Satheesh
Shreyas Kaushik wrote:
> I guess this is an IDE trick , I'll change this.
>
> ~ Shreyas
>
> Army wrote:
>
>> <Sh...@sun.com> wrote
>>
>>>
>>> -import java.sql.Date;
>>> -import java.sql.Time;
>>> -import java.sql.Timestamp;
>>> -import java.sql.Types;
>>> -import java.sql.ResultSet;
>>> -import java.sql.SQLException;
>>> +import java.sql.*;
>>
>>
>>
>> I apologize if this is overly picky, but isn't inclusion of a package
>> via the "*" wildcard generally considered an unfavorable programming
>> practice? I think it's fine to do so when writing tests, but as far
>> as I know, the Derby codeline generally avoids using wildcard
>> imports. And even if there are existing places where such imports
>> are used, it seems to me that _replacing_ existing imports with the
>> wildcard import is probably not a habit to encourage.
>>
>> Even if functionally speaking there's no difference, to do this is to
>> introduce a style of imports that's doesn't agree with the rest of
>> the codeline--is that something to avoid?
>>
>> 'Course, maybe that's just unfounded paranoia on my part...
>>
>> *shrug*
>> Army
>>
>
>
>
Re: [PATCH] Derby-174
Posted by Shreyas Kaushik <Sh...@Sun.COM>.
I guess this is an IDE trick , I'll change this.
~ Shreyas
Army wrote:
> <Sh...@sun.com> wrote
>
>>
>> -import java.sql.Date;
>> -import java.sql.Time;
>> -import java.sql.Timestamp;
>> -import java.sql.Types;
>> -import java.sql.ResultSet;
>> -import java.sql.SQLException;
>> +import java.sql.*;
>
>
> I apologize if this is overly picky, but isn't inclusion of a package
> via the "*" wildcard generally considered an unfavorable programming
> practice? I think it's fine to do so when writing tests, but as far
> as I know, the Derby codeline generally avoids using wildcard
> imports. And even if there are existing places where such imports are
> used, it seems to me that _replacing_ existing imports with the
> wildcard import is probably not a habit to encourage.
>
> Even if functionally speaking there's no difference, to do this is to
> introduce a style of imports that's doesn't agree with the rest of the
> codeline--is that something to avoid?
>
> 'Course, maybe that's just unfounded paranoia on my part...
>
> *shrug*
> Army
>
Re: [PATCH] Derby-174
Posted by Army <qo...@sbcglobal.net>.
<Sh...@sun.com> wrote
>
>-import java.sql.Date;
>-import java.sql.Time;
>-import java.sql.Timestamp;
>-import java.sql.Types;
>-import java.sql.ResultSet;
>-import java.sql.SQLException;
>+import java.sql.*;
I apologize if this is overly picky, but isn't inclusion of a package via the "*" wildcard generally considered an
unfavorable programming practice? I think it's fine to do so when writing tests, but as far as I know, the Derby
codeline generally avoids using wildcard imports. And even if there are existing places where such imports are used, it
seems to me that _replacing_ existing imports with the wildcard import is probably not a habit to encourage.
Even if functionally speaking there's no difference, to do this is to introduce a style of imports that's doesn't agree
with the rest of the codeline--is that something to avoid?
'Course, maybe that's just unfounded paranoia on my part...
*shrug*
Army
Re: [PATCH] Derby-174
Posted by Shreyas Kaushik <Sh...@sun.com>.
Hi Mamta,
Thanks for your comments.
Currently the *setInto* method is not overridden in SQLTimestamp.java,
hence it uses the one defined in DataType.java which is as shown below,
public void setInto(PreparedStatement ps, int position) throws
SQLException, StandardException {
ps.setObject(position, getObject());
}
Here there is a call to getObject() which is overridden in
SQLTimestamp.java which calls getTimestamp((Calendar)null); which
returns null.
Since the setObject cannot take an utyped null ( according to JDBC 3.0
spec ) it throws an Exception.
So calling setInto in this case is equivalent to calling
ps.setTimestamp(position, getTimestamp((Calendar) null));
So whenever setInto is called on a Timestamp it is as if I am calling the above code.
~ Shreyas
Mamta Satoor wrote:
>On Tue, 22 Mar 2005 13:33:25 +0530, Shreyas Kaushik
><Sh...@sun.com> wrote:
>
>
>>Find the patch for this attached. I will send the test patch in a
>>seperate mail.
>>
>>~ Shreyas
>>
>>
>>Index: java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
>>===================================================================
>>--- java/engine/org/apache/derby/iapi/types/SQLTimestamp.java (revision 158017)
>>+++ java/engine/org/apache/derby/iapi/types/SQLTimestamp.java (working copy)
>>@@ -45,12 +45,7 @@
>>import org.apache.derby.iapi.types.SQLDouble;
>>import org.apache.derby.iapi.types.SQLTime;
>>
>>-import java.sql.Date;
>>-import java.sql.Time;
>>-import java.sql.Timestamp;
>>-import java.sql.Types;
>>-import java.sql.ResultSet;
>>-import java.sql.SQLException;
>>+import java.sql.*;
>>
>>import java.util.Calendar;
>>import java.util.GregorianCalendar;
>>@@ -989,5 +984,10 @@
>> currentCal.setTime(value);
>> return SQLTime.computeEncodedTime(currentCal);
>> }
>>+
>>+ public void setInto(PreparedStatement ps, int position) throws SQLException, StandardException {
>>+
>>+ ps.setTimestamp(position, getTimestamp((Calendar) null));
>>+ }
>>}
>>
>>
>>
>>
>>
>
>Hi Shreyas,
>
>Haven't looked into details of current implementation of setInto
>methods by other datatypes, but I did notice that most other datatypes
>have coded setInto method as follows
>//this method is from SQLDecimal.java
>public final void setInto(PreparedStatement ps, int position) throws
>SQLException {
> if (isNull()) {
> ps.setNull(position, java.sql.Types.DECIMAL);
> return;
> }
> ps.setBigDecimal(position, getBigDecimal());
>}
>
>Seems like they first check for isNULL and call different methods
>depending on whether the method is dealing with null or not. Wondered
>why your patch does not check for null before calling ps.setTimestamp
>method?
>
>Mamta
>
>
Re: [PATCH] Derby-174
Posted by Mamta Satoor <ms...@gmail.com>.
On Tue, 22 Mar 2005 13:33:25 +0530, Shreyas Kaushik
<Sh...@sun.com> wrote:
> Find the patch for this attached. I will send the test patch in a
> seperate mail.
>
> ~ Shreyas
>
>
> Index: java/engine/org/apache/derby/iapi/types/SQLTimestamp.java
> ===================================================================
> --- java/engine/org/apache/derby/iapi/types/SQLTimestamp.java (revision 158017)
> +++ java/engine/org/apache/derby/iapi/types/SQLTimestamp.java (working copy)
> @@ -45,12 +45,7 @@
> import org.apache.derby.iapi.types.SQLDouble;
> import org.apache.derby.iapi.types.SQLTime;
>
> -import java.sql.Date;
> -import java.sql.Time;
> -import java.sql.Timestamp;
> -import java.sql.Types;
> -import java.sql.ResultSet;
> -import java.sql.SQLException;
> +import java.sql.*;
>
> import java.util.Calendar;
> import java.util.GregorianCalendar;
> @@ -989,5 +984,10 @@
> currentCal.setTime(value);
> return SQLTime.computeEncodedTime(currentCal);
> }
> +
> + public void setInto(PreparedStatement ps, int position) throws SQLException, StandardException {
> +
> + ps.setTimestamp(position, getTimestamp((Calendar) null));
> + }
> }
>
>
>
Hi Shreyas,
Haven't looked into details of current implementation of setInto
methods by other datatypes, but I did notice that most other datatypes
have coded setInto method as follows
//this method is from SQLDecimal.java
public final void setInto(PreparedStatement ps, int position) throws
SQLException {
if (isNull()) {
ps.setNull(position, java.sql.Types.DECIMAL);
return;
}
ps.setBigDecimal(position, getBigDecimal());
}
Seems like they first check for isNULL and call different methods
depending on whether the method is dealing with null or not. Wondered
why your patch does not check for null before calling ps.setTimestamp
method?
Mamta