You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ojb-dev@db.apache.org by Armin Waibel <ar...@code-au-lait.de> on 2003/11/21 22:07:42 UTC

[VOTE] Design bug fixed - check in?

Hi all,

approximately 10 days ago I posted a mail concerning
a "design bug" in OJB-Kernel api
("Kernel api changes be necessary!?").
The problem was the loss of persistent field
metadata information from PB to the database operation
classes.

For examples: When storing an object sooner or
later an array of object field values were passed
to StatementManager. Then the StatementManager lookup the
jdbcTypes using SqlHelper class, because we pass the
field value array without any metadata information.
But we know the jdbcTypes from the metadata. Thus if
we pass a ValueContainer class
ValueContainer
{
Object value;
int jdbcType;
}

instead of the pure value object, we no longer need
many of SqlHelper/SqlTypeHelper methods.

Another problem is the support of BOOLEAN datatype.
If the given object in SqlHelper#getSqlTypeByValue(Object value)
is instance of Boolean always Types.BIT instead of Types.Boolean
is returned. So, there is no way to support Type.BOOLEAN
with current implementation.

Local I made the described refactoring, running the test
suite give me the same result as the CVS head, also
the performance test (perf-test task).

I had to change methods of two intern kernel interfaces
- JdbcAccess and StatementManagerIF.

StatementManagerIF:
<     int bindValues(PreparedStatement stmt, Object[] values, int index) 
throws SQLException;
---
 >     int bindValues(PreparedStatement stmt, ValueContainer[] 
valueContainer, int index) throws SQLException;

JdbcAccess:
<     public ResultSetAndStatement executeSQL(String sqlStatement, 
ClassDescriptor cld, Object[] values, boolean scrollable) throws 
PersistenceBrokerException;
---
 >     public ResultSetAndStatement executeSQL(String sqlStatement, 
ClassDescriptor cld, ValueContainer[] values, boolean scrollable) throws 
PersistenceBrokerException;
129c130
<     public int executeUpdateSQL(String sqlStatement, ClassDescriptor 
cld, Object[] values1, Object[] values2)
---
 >     public int executeUpdateSQL(String sqlStatement, ClassDescriptor 
cld, ValueContainer[] values1, ValueContainer[] values2)


By the way, I split PBImpl class (~3000 loc! monster) into
PBImpl (~ 1700)
MtoNBroker(~400 loc)
QueryReferenceBroker (~900)
this should help us to keep track of source.

I propose to accept the "design fix" and refactoring.
What do you think? Comments?


regards,
Armin



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Thomas Mahler <th...@web.de>.
I'm +1 for the change.
I agree with Armin that these are important fixes that should be 
available in 1.o and not later!

cu,
thomas

Armin Waibel wrote:
> Hi all,
> 
> approximately 10 days ago I posted a mail concerning
> a "design bug" in OJB-Kernel api
> ("Kernel api changes be necessary!?").
> The problem was the loss of persistent field
> metadata information from PB to the database operation
> classes.
> 
> For examples: When storing an object sooner or
> later an array of object field values were passed
> to StatementManager. Then the StatementManager lookup the
> jdbcTypes using SqlHelper class, because we pass the
> field value array without any metadata information.
> But we know the jdbcTypes from the metadata. Thus if
> we pass a ValueContainer class
> ValueContainer
> {
> Object value;
> int jdbcType;
> }
> 
> instead of the pure value object, we no longer need
> many of SqlHelper/SqlTypeHelper methods.
> 
> Another problem is the support of BOOLEAN datatype.
> If the given object in SqlHelper#getSqlTypeByValue(Object value)
> is instance of Boolean always Types.BIT instead of Types.Boolean
> is returned. So, there is no way to support Type.BOOLEAN
> with current implementation.
> 
> Local I made the described refactoring, running the test
> suite give me the same result as the CVS head, also
> the performance test (perf-test task).
> 
> I had to change methods of two intern kernel interfaces
> - JdbcAccess and StatementManagerIF.
> 
> StatementManagerIF:
> <     int bindValues(PreparedStatement stmt, Object[] values, int index) 
> throws SQLException;
> ---
>  >     int bindValues(PreparedStatement stmt, ValueContainer[] 
> valueContainer, int index) throws SQLException;
> 
> JdbcAccess:
> <     public ResultSetAndStatement executeSQL(String sqlStatement, 
> ClassDescriptor cld, Object[] values, boolean scrollable) throws 
> PersistenceBrokerException;
> ---
>  >     public ResultSetAndStatement executeSQL(String sqlStatement, 
> ClassDescriptor cld, ValueContainer[] values, boolean scrollable) throws 
> PersistenceBrokerException;
> 129c130
> <     public int executeUpdateSQL(String sqlStatement, ClassDescriptor 
> cld, Object[] values1, Object[] values2)
> ---
>  >     public int executeUpdateSQL(String sqlStatement, ClassDescriptor 
> cld, ValueContainer[] values1, ValueContainer[] values2)
> 
> 
> By the way, I split PBImpl class (~3000 loc! monster) into
> PBImpl (~ 1700)
> MtoNBroker(~400 loc)
> QueryReferenceBroker (~900)
> this should help us to keep track of source.
> 
> I propose to accept the "design fix" and refactoring.
> What do you think? Comments?
> 
> 
> regards,
> Armin
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Jakob Braeuchi <jb...@gmx.ch>.
hi armin,

i do not like api-changes in release-candidates. but if it's 
_absolutely_ necessary to fix a bug, i think we should nonetheless do it.

jakob

Armin Waibel wrote:

> Hi all,
> 
> approximately 10 days ago I posted a mail concerning
> a "design bug" in OJB-Kernel api
> ("Kernel api changes be necessary!?").
> The problem was the loss of persistent field
> metadata information from PB to the database operation
> classes.
> 
> For examples: When storing an object sooner or
> later an array of object field values were passed
> to StatementManager. Then the StatementManager lookup the
> jdbcTypes using SqlHelper class, because we pass the
> field value array without any metadata information.
> But we know the jdbcTypes from the metadata. Thus if
> we pass a ValueContainer class
> ValueContainer
> {
> Object value;
> int jdbcType;
> }
> 
> instead of the pure value object, we no longer need
> many of SqlHelper/SqlTypeHelper methods.
> 
> Another problem is the support of BOOLEAN datatype.
> If the given object in SqlHelper#getSqlTypeByValue(Object value)
> is instance of Boolean always Types.BIT instead of Types.Boolean
> is returned. So, there is no way to support Type.BOOLEAN
> with current implementation.
> 
> Local I made the described refactoring, running the test
> suite give me the same result as the CVS head, also
> the performance test (perf-test task).
> 
> I had to change methods of two intern kernel interfaces
> - JdbcAccess and StatementManagerIF.
> 
> StatementManagerIF:
> <     int bindValues(PreparedStatement stmt, Object[] values, int index) 
> throws SQLException;
> ---
>  >     int bindValues(PreparedStatement stmt, ValueContainer[] 
> valueContainer, int index) throws SQLException;
> 
> JdbcAccess:
> <     public ResultSetAndStatement executeSQL(String sqlStatement, 
> ClassDescriptor cld, Object[] values, boolean scrollable) throws 
> PersistenceBrokerException;
> ---
>  >     public ResultSetAndStatement executeSQL(String sqlStatement, 
> ClassDescriptor cld, ValueContainer[] values, boolean scrollable) throws 
> PersistenceBrokerException;
> 129c130
> <     public int executeUpdateSQL(String sqlStatement, ClassDescriptor 
> cld, Object[] values1, Object[] values2)
> ---
>  >     public int executeUpdateSQL(String sqlStatement, ClassDescriptor 
> cld, ValueContainer[] values1, ValueContainer[] values2)
> 
> 
> By the way, I split PBImpl class (~3000 loc! monster) into
> PBImpl (~ 1700)
> MtoNBroker(~400 loc)
> QueryReferenceBroker (~900)
> this should help us to keep track of source.
> 
> I propose to accept the "design fix" and refactoring.
> What do you think? Comments?
> 
> 
> regards,
> Armin
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Brian McCallister <mc...@forthillcompany.com>.
On Friday, November 21, 2003, at 05:11 PM, Armin Waibel wrote:
>> , but I really don't like significant interface changes while in rc.
> Are they really significant?
>

I don't use any of them -- but i don't know who does.

A possible route is to deprecate things for 1.0 but not yet remove, 
then remove in 1.1.

-Brian



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Brian McCallister <mc...@forthillcompany.com>.
On Friday, November 21, 2003, at 05:11 PM, Armin Waibel wrote:

> Hi again,
> I will explain one of the superfluously (bad) methods
> of SqlHelper/SqlTypeHelper more detailed...
> <snip>

Okay, I am convinced =)

+1

-Brian



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Armin Waibel <ar...@code-au-lait.de>.
Hi again,

Brian McCallister wrote:

>> If we don't accept these changes, the first day after 1.0
>> release we will have two completely different branches.
>>
>> For me these changes are a step forward to 1.0
> 
> 
> I would post to the -users list about this, in all seriousness. At least 
> three people *just at ApacheCon* mentioned how annoying interface 
> changes are in rc candidates.
>
Who are these guys? ;-)
Agree completely. But much more annoying is a product that
does not what you expected.

I will explain one of the superfluously (bad) methods
of SqlHelper/SqlTypeHelper more detailed

public static int getSqlTypeByValue(Object value)
{
...
else if (value instanceof Boolean)
         {
             return Types.BIT;
         }
If the field value is a boolean always BIT
is returned. But jdbc type BIT and BOOLEAN
can be mapped to a java boolean


...

else if (value instanceof byte[])
         {
             return Types.VARBINARY;
         }
But BINARY, VARBINARY, LONGVARBINARY can
be mapped to byte[]

...
else if (value instanceof String)
         {
             return Types.VARCHAR;
         }
But CHAR, VARCHAR, LONGVARCHAR can be mapped
to String.

So, all this metadata information is lost with current
version. I'm not a database guy, thus I don't know if
this could cause side-effects ...

> I agree that the changes should be made, and once 1.0 is out some even 
> crazier changes which I am sitting on at the moment
sound auspicious ;-)

>, but I really don't 
> like significant interface changes while in rc.
> 
Are they really significant?

regards,
Armin

> A branch at 1.0 is *fine* that is the idea.
> 
> -Brian
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
> 
> 
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Brian McCallister <mc...@forthillcompany.com>.
On Friday, November 21, 2003, at 04:33 PM, Armin Waibel wrote:

> Hi Brian,
>
> I just read your blog about the ApacheCon
> really interesting! Seems you talked to everyone ;-)

Was really great to meet people who have otherwise been names on email 
or in irc =)

> If we don't accept these changes, the first day after 1.0
> release we will have two completely different branches.
>
> For me these changes are a step forward to 1.0

I would post to the -users list about this, in all seriousness. At 
least three people *just at ApacheCon* mentioned how annoying interface 
changes are in rc candidates.

I agree that the changes should be made, and once 1.0 is out some even 
crazier changes which I am sitting on at the moment, but I really don't 
like significant interface changes while in rc.

A branch at 1.0 is *fine* that is the idea.

-Brian



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Armin Waibel <ar...@code-au-lait.de>.
Hi Brian,

I just read your blog about the ApacheCon
really interesting! Seems you talked to everyone ;-)

Brian McCallister wrote:

> On Friday, November 21, 2003, at 04:07 PM, Armin Waibel wrote:
> 
>> I propose to accept the "design fix" and refactoring.
>> What do you think? Comments?
> 
> 
> Are we really sure we want to change the API at rc4? It might be better 
> to do this post 1.0
> 
I know it's not a good karma modify interfaces in any
rc candidate. But from my point of view it's a bug fix
to support JDBC 3.0 datatype BOOLEAN.

If we don't accept these changes, the first day after 1.0
release we will have two completely different branches.

For me these changes are a step forward to 1.0

regards,
Armin

> -Brian
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
> 
> 
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Re: [VOTE] Design bug fixed - check in?

Posted by Brian McCallister <mc...@forthillcompany.com>.
On Friday, November 21, 2003, at 04:07 PM, Armin Waibel wrote:
> I propose to accept the "design fix" and refactoring.
> What do you think? Comments?

Are we really sure we want to change the API at rc4? It might be better 
to do this post 1.0

-Brian



---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org