[dbutils] Java5 branch landed to trunk; release tomorrow?

[Dan Fabulich]

In r832220 I merged the sandbox/dbutils/java5 branch to
proper/dbutils/trunk.  The build works and has no compiler warnings.

I've also fixed bug DBUTILS-58, which provides a workaround for bug
DBUTILS-56.

https://issues.apache.org/jira/browse/DBUTILS-56 
https://issues.apache.org/jira/browse/DBUTILS-58

At this point, I think I'd be happy to cut a release...  I'll fire up an
RC at ApacheCon Hackathon tomorrow unless somebody (Julien?) identifies
some other bugs that need fixing.

-Dan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Julien Aymé]

I've submitted a patch for DBUTILS-54 and DBUTILS-57, which can be
added to the release if someone (Dan? ;-) ) has the time to review
them.

I'll try to submit a patch for DBUTILS-50 later this evening, but I
think this will require a little more time and thoughts.

https://issues.apache.org/jira/browse/DBUTILS-54
https://issues.apache.org/jira/browse/DBUTILS-57
https://issues.apache.org/jira/browse/DBUTILS-50

Anyway, thanks for pushing this release.

Regards,
Julien

2009/11/3 Dan Fabulich <[hidden email]>:

>
> In r832220 I merged the sandbox/dbutils/java5 branch to
> proper/dbutils/trunk.  The build works and has no compiler warnings.
>
> I've also fixed bug DBUTILS-58, which provides a workaround for bug
> DBUTILS-56.
>
> https://issues.apache.org/jira/browse/DBUTILS-56
> https://issues.apache.org/jira/browse/DBUTILS-58
>
> At this point, I think I'd be happy to cut a release...  I'll fire up an RC
> at ApacheCon Hackathon tomorrow unless somebody (Julien?) identifies some
> other bugs that need fixing.
>
> -Dan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jörg Schaible]

Dan Fabulich wrote at Dienstag, 3. November 2009 02:57:

>
> In r832220 I merged the sandbox/dbutils/java5 branch to
> proper/dbutils/trunk.  The build works and has no compiler warnings.

Is it binary backward compatible?

- Jörg


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Dan Fabulich]

Jörg Schaible wrote:

> Is it binary backward compatible?

I think it is, but I'm not 100% certain; it's confusing.

I ran clirr on dbutils-1.2 and my locally built 1.3-SNAPSHOT; it found one
clear compatibility error which I just fixed (DBUTILS-61).  This is what
it says as of r832529:

INFO: 7011: org.apache.commons.dbutils.ProxyFactory: Method 'public java.lang.Object newProxyInstance(java.lang.Class, java.lang.reflect.InvocationHandler)' has been added
INFO: 7011: org.apache.commons.dbutils.ResultSetIterator: Method 'public java.lang.Iterable iterable(java.sql.ResultSet)' has been added
ERROR: 7006: org.apache.commons.dbutils.ResultSetIterator: Return type of method 'public java.lang.Object next()' has been changed to java.lang.Object[]
INFO: 7011: org.apache.commons.dbutils.ResultSetIterator: Method 'public java.lang.Object next()' has been added
INFO: 8000: org.apache.commons.dbutils.handlers.AbstractKeyedHandler: Class org.apache.commons.dbutils.handlers.AbstractKeyedHandler added
ERROR: 7006: org.apache.commons.dbutils.handlers.AbstractListHandler: Return type of method 'public java.lang.Object handle(java.sql.ResultSet)' has been changed to java.util.List
INFO: 7011: org.apache.commons.dbutils.handlers.AbstractListHandler: Method 'public java.lang.Object handle(java.sql.ResultSet)' has been added
ERROR: 7006: org.apache.commons.dbutils.handlers.ArrayHandler: Return type of method 'public java.lang.Object handle(java.sql.ResultSet)' has been changed to java.lang.Object[]
INFO: 7011: org.apache.commons.dbutils.handlers.ArrayHandler: Method 'public java.lang.Object handle(java.sql.ResultSet)' has been added
ERROR: 7006: org.apache.commons.dbutils.handlers.ArrayListHandler: Return type of method 'protected java.lang.Object handleRow(java.sql.ResultSet)' has been changed to java.lang.Object[]
INFO: 7011: org.apache.commons.dbutils.handlers.ArrayListHandler: Method 'protected java.lang.Object handleRow(java.sql.ResultSet)' has been added
ERROR: 7006: org.apache.commons.dbutils.handlers.BeanListHandler: Return type of method 'public java.lang.Object handle(java.sql.ResultSet)' has been changed to java.util.List
INFO: 7011: org.apache.commons.dbutils.handlers.BeanListHandler: Method 'public java.lang.Object handle(java.sql.ResultSet)' has been added
INFO: 5000: org.apache.commons.dbutils.handlers.KeyedHandler: Added org.apache.commons.dbutils.handlers.AbstractKeyedHandler to the list of superclasses
INFO: 7000: org.apache.commons.dbutils.handlers.KeyedHandler: Method 'protected java.util.Map createMap()' is now implemented in superclass org.apache.commons.dbutils.handlers.AbstractKeyedHandler
ERROR: 7006: org.apache.commons.dbutils.handlers.KeyedHandler: Return type of method 'protected java.lang.Object createRow(java.sql.ResultSet)' has been changed to java.util.Map
INFO: 7011: org.apache.commons.dbutils.handlers.KeyedHandler: Method 'protected java.lang.Object createRow(java.sql.ResultSet)' has been added
INFO: 7003: org.apache.commons.dbutils.handlers.KeyedHandler: Method 'public java.lang.Object handle(java.sql.ResultSet)' has been removed, but an inherited definition exists.
ERROR: 7006: org.apache.commons.dbutils.handlers.MapHandler: Return type of method 'public java.lang.Object handle(java.sql.ResultSet)' has been changed to java.util.Map
INFO: 7011: org.apache.commons.dbutils.handlers.MapHandler: Method 'public java.lang.Object handle(java.sql.ResultSet)' has been added
ERROR: 7006: org.apache.commons.dbutils.handlers.MapListHandler: Return type of method 'protected java.lang.Object handleRow(java.sql.ResultSet)' has been changed to java.util.Map
INFO: 7011: org.apache.commons.dbutils.handlers.MapListHandler: Method 'protected java.lang.Object handleRow(java.sql.ResultSet)' has been added

All of the errors are complaints about covariant return types. In each
case, something that used to return an Object now returns a more specific
type (e.g. List or Object[]).

But it's confusing, because Java handles covariant return types by
generating replacement methods in the bytecode.  So there IS still a
method that returns Object in the bytecode.

For example, according to Jad, BeanListHandler.class decompiles like this:

   public List handle(ResultSet rs) throws SQLException {
     return convert.toBeanList(rs, type);
   }

   public volatile Object handle(ResultSet resultset) throws SQLException {
     return handle(resultset);
   }

Strangely, Clirr is reporting this as an ERROR that the method has
changed, followed by an INFO remark that a new method was added that just
happens to be exactly like the old method!  (I guess the new signature
isn't exactly the same, since it is marked "volatile" ...?)

I believe none of these errors are really errors.  I tested this by
creating a subclass of KeyedHandler like this:

   public class MyKeyedHandler extends KeyedHandler {
     public boolean hit = false;
     @Override
     protected Object createRow(ResultSet rs) throws SQLException {
       hit = true;
       return super.createRow(rs);
     }
   }

I used that to create a jar called "backcompat.jar"

I then copied KeyedHandlerTest from 1.2 and changed it to refer to
MyKeyedHandler.  I also added assertions that "hit" turned to true when it
was invoked.  I ran the test against 1.2 and backcompat.jar and it passed;
then I replaced the 1.2 jar with the 1.3 jar and it again passed, with no
changes to backcompat.jar.

So it seems to me that all of these errors aren't really errors, but
rather bugs in clirr.

-Dan



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Dan Fabulich]

Julien Aymé wrote:

> I've submitted a patch for DBUTILS-54 and DBUTILS-57, which can be
> added to the release if someone (Dan? ;-) ) has the time to review
> them.
>
> I'll try to submit a patch for DBUTILS-50 later this evening, but I
> think this will require a little more time and thoughts.
>
> https://issues.apache.org/jira/browse/DBUTILS-54
> https://issues.apache.org/jira/browse/DBUTILS-57
> https://issues.apache.org/jira/browse/DBUTILS-50

I submitted your patch for DBUTILS-57, but I -1 the patch for DBUTIL-54...
it's just too much complexity for a "Minor" issue.

I think DBUTILS-50 will also be controversial (Liam had argued against it)
so at this point I'd like to move ahead with a release candidate for 1.3.
DBUTILS-54 and DBUTILS-50 can be handled in a later release, IMO.  (I'd
like to do the release this week; I've blocked out time during ApacheCon
to think about this.)

-Dan


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Julien Aymé]

Thanks for the review!

2009/11/4 Dan Fabulich <[hidden email]>:

> Julien Aymé wrote:
>
>> I've submitted a patch for DBUTILS-54 and DBUTILS-57, which can be
>> added to the release if someone (Dan? ;-) ) has the time to review
>> them.
>>
>> I'll try to submit a patch for DBUTILS-50 later this evening, but I
>> think this will require a little more time and thoughts.
>>
>> https://issues.apache.org/jira/browse/DBUTILS-54
>> https://issues.apache.org/jira/browse/DBUTILS-57
>> https://issues.apache.org/jira/browse/DBUTILS-50
>
> I submitted your patch for DBUTILS-57, but I -1 the patch for DBUTIL-54...
> it's just too much complexity for a "Minor" issue.

</snip>
I thought so when I wrote the patch for DBUTILS-54. I wasn't completly
satisfied with it, since the patch is really long and add too many methods in
QueryRunner. Maybe we should rewrite it in another specialized class?
Anyway, +1 to leave it open for the moment.

>
> I think DBUTILS-50 will also be controversial (Liam had argued against it)
> so at this point I'd like to move ahead with a release candidate for 1.3.
> DBUTILS-54 and DBUTILS-50 can be handled in a later release, IMO.  (I'd like
> to do the release this week; I've blocked out time during ApacheCon to think
> about this.)

</snap>
I'm totally OK with it, please do. And I also agree with Liam, DBUTILS-50 is
really complicated to deal with. I tried to implement it in a simple yet usefull
way and I couldn't achieve what I wanted.
So +1 to leave it open and fix later.


Julien

>
> -Dan
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jörg Schaible]

Dan Fabulich wrote at Dienstag, 3. November 2009 21:03:

> Jörg Schaible wrote:
>
>> Is it binary backward compatible?
>
> I think it is, but I'm not 100% certain; it's confusing.
>
> I ran clirr on dbutils-1.2 and my locally built 1.3-SNAPSHOT; it found one
> clear compatibility error which I just fixed (DBUTILS-61).  This is what
> it says as of r832529:

[snip]

> All of the errors are complaints about covariant return types. In each
> case, something that used to return an Object now returns a more specific
> type (e.g. List or Object[]).
>
> But it's confusing, because Java handles covariant return types by
> generating replacement methods in the bytecode.  So there IS still a
> method that returns Object in the bytecode.
>
> For example, according to Jad, BeanListHandler.class decompiles like this:
>
>    public List handle(ResultSet rs) throws SQLException {
>      return convert.toBeanList(rs, type);
>    }
>
>    public volatile Object handle(ResultSet resultset) throws SQLException
>    {
>      return handle(resultset);
>    }
>
> Strangely, Clirr is reporting this as an ERROR that the method has
> changed, followed by an INFO remark that a new method was added that just
> happens to be exactly like the old method!  (I guess the new signature
> isn't exactly the same, since it is marked "volatile" ...?)
>
> I believe none of these errors are really errors.  I tested this by
> creating a subclass of KeyedHandler like this:
>
>    public class MyKeyedHandler extends KeyedHandler {
>      public boolean hit = false;
>      @Override
>      protected Object createRow(ResultSet rs) throws SQLException {
>        hit = true;
>        return super.createRow(rs);
>      }
>    }
>
> I used that to create a jar called "backcompat.jar"
>
> I then copied KeyedHandlerTest from 1.2 and changed it to refer to
> MyKeyedHandler.  I also added assertions that "hit" turned to true when it
> was invoked.  I ran the test against 1.2 and backcompat.jar and it passed;
> then I replaced the 1.2 jar with the 1.3 jar and it again passed, with no
> changes to backcompat.jar.
>
> So it seems to me that all of these errors aren't really errors, but
> rather bugs in clirr.

Maybe you can use the src tarball from 1.2, change group and artifact id to
something local, drop the Java source from main and add 1.3-SNAPSHOT as
dependency. Then you should be able to run with this POM the tests of 1.2
with code base of 1.3. I simply have currently no svn access (internal
problem) to checkout dbutils trunk otherwise I'd done this myself.

I think your conclusion is right, but we should be sure before releasing 1.3
as possible drop in for older versions.

- Jörg


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Dan Fabulich]

Jörg Schaible wrote:

> Maybe you can use the src tarball from 1.2, change group and artifact id to
> something local, drop the Java source from main and add 1.3-SNAPSHOT as
> dependency. Then you should be able to run with this POM the tests of 1.2
> with code base of 1.3.

That test passes, though I don't think that's technically a test of binary
compatibility, since it's compiling the tests against 1.3.

So for my second test I did "mvn clean test" on the 1.2 source, then
deleted the "classes" directory and added 1.3 as a dependency, so the
previously compiled test classes would run against 1.3 binaries.  That
test passed, too.

I think we're good!

-Dan


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

[Jörg Schaible]

Dan Fabulich wrote at Mittwoch, 4. November 2009 18:18:

> Jörg Schaible wrote:
>
>> Maybe you can use the src tarball from 1.2, change group and artifact id
>> to something local, drop the Java source from main and add 1.3-SNAPSHOT
>> as dependency. Then you should be able to run with this POM the tests of
>> 1.2 with code base of 1.3.
>
> That test passes, though I don't think that's technically a test of binary
> compatibility, since it's compiling the tests against 1.3.
>
> So for my second test I did "mvn clean test" on the 1.2 source, then
> deleted the "classes" directory and added 1.3 as a dependency, so the
> previously compiled test classes would run against 1.3 binaries.  That
> test passed, too.
>
> I think we're good!

That sounds really good!

- Jörg


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]