You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ojb-user@db.apache.org by Sascha Broich <sa...@tsa.de> on 2008/03/06 10:47:39 UTC
Bugs in 1.0.5rc1
Hello,
I found a bug in
org.apache.ojb.broker.core.CollectionTypes#getCollectionClass(Collection
Descriptor).
The "isAssignableFrom" check has to be inverted.
For instance:
HashSet.class.isAssignableFrom(Set.class) yields false and a
MetadataException is thrown.
Inverted to Set.class.isAssignableFrom(HashSet.class) yields true and
the correct branch gets entered.
Another bug is in the statement creation when a subclass is involved.
Note: A0 is the subclass table from A1, PHB_USRID exists only in A1
1.0.4 creates something like
SELECT
A0.PHB_ID ,
A1.PHB_USRID ,
FROM
IPT_PHONEBOOKCSV A0
INNER JOIN IPT_PHONEBOOK A1
ON
A0.PHB_ID = A1.PHB_ID
WHERE
PHB_USRID IS NULL
1.0.5rc1 creates
SELECT
A0.PHB_ID ,
A1.PHB_ID ,
A1.PHB_USRID ,
FROM
IPT_PHONEBOOKCSV A0
INNER JOIN IPT_PHONEBOOK A1
ON
A0.PHB_ID = A1.PHB_ID
WHERE
A0.PHB_USRID IS NULL
The WHERE criteria assignment to A0 the statement produces an exception
for the nonexisting column A0.PHB_USRID.
Note, that in the SELECT of 1.0.5rc1 is also A1.PHB_ID, which is not in
the statement of 1.0.4.
Regards,
Sascha Broich
---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-user-help@db.apache.org
AW: Bugs in 1.0.5rc1
Posted by Sascha Broich <sa...@tsa.de>.
Hi Armin,
> Below you can find a new (backward compatible) version of method
> CollectionTypes#getCollectionClass. Give it a try.
Thanks, it seems to work.
Regards,
Sascha Broich
---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-user-help@db.apache.org
Re: Bugs in 1.0.5rc1
Posted by Armin Waibel <ar...@apache.org>.
Hi Sascha,
Sascha Broich wrote:
> Hi Armin
>
>> yep, this is the intended behavior. This method (beside user specific
>> ManageableCollection implementations) only allow base collection
>> classes
>> (List, Set,...).
>> The mentioned line is similar to:
>> Set.class.equals(fieldClass)
>
> Okay, if this is intended.
> It was just a stumbling block.
> I will look if I can change our code with backward compatibility.
Below you can find a new (backward compatible) version of method
CollectionTypes#getCollectionClass. Give it a try.
>
>
>> You are right, this should not happen. In the OJB test-suite we do
>> several tests using table-per-subclass inheritance and this never
>> happens.
>> Could you post the mapping for class A1, A0 and the source of the
> query
>> then I can try to reproduce your issue.
>
> The criteria for this issue was build by using the table column name
> 'PHB_USRID'.
> After I changed this to 'addIsNull("m_iUserId") the query succeeded.
Thanks for the detailed description. Now I'm able to reproduce the bug.
I setup a test using table-per-subclass inheritance. This test search
for a nullified field using a field attribute and then a column attribute:
Criteria c1 = new Criteria()
.addEqualTo("id_2", id_2)
.addIsNull("name");
Criteria c2 = new Criteria()
.addEqualTo("id_2", id_2)
.addColumnIsNull("NAME");
QueryByCriteria q1 = QueryFactory.newQuery(Executive.class, c1, true);
QueryByCriteria q2 = QueryFactory.newQuery(Executive.class, c2, true);
The first query pass, the second returns the same error you described in
a previous mail. Will try to fix this ASAP!
regards,
Armin
new version of method CollectionTypes#getCollectionClass
-------------------------------------------------------
// configurable property
private boolean optimized = true;
public Class getCollectionClass(CollectionDescriptor cds) throws
PersistenceBrokerException
{
Class result = cds.getCollectionClass();
if(result == null)
{
boolean failed = false;
Class fieldType = cds.getPersistentField().getType();
/*
first check for user specific ManageableCollection type,
then check for base collection types (List, Set, Vector,
Collection)
and check if the collection field type is assignable from the
predefined
collection type implementation (predefined/configured
implemenations of ManageableCollection).
*/
if (ManageableCollection.class.isAssignableFrom(fieldType))
{
result = fieldType;
}
else if(Vector.class.isAssignableFrom(fieldType))
{
result = cds.isMtoNRelation() ? getManyToManyVector() :
getOneToManyVector();
if(!fieldType.isAssignableFrom(result))
{
failed = true;
}
}
else if(List.class.isAssignableFrom(fieldType))
{
result = cds.isMtoNRelation() ? getManyToManyList() :
getOneToManyList();
if(!fieldType.isAssignableFrom(result))
{
failed = true;
}
}
else if(Set.class.isAssignableFrom(fieldType))
{
result = cds.isMtoNRelation() ? getManyToManySet() :
getOneToManySet();
if(!fieldType.isAssignableFrom(result))
{
failed = true;
}
}
else if(Collection.class.isAssignableFrom(fieldType))
{
result = cds.isMtoNRelation() ? getManyToManyCollection() :
getOneToManyCollection();
if(!fieldType.isAssignableFrom(result))
{
failed = true;
}
}
else if(fieldType.isArray())
{
result = cds.isMtoNRelation() ? getManyToManyArray() :
getOneToManyArray();
}
else
{
failed = true;
}
if(failed)
{
throw new MetadataException(
"Cannot determine a collection type for
collection/list/set/array field '"
+ cds.getAttributeName() + "' of type '" +
fieldType + "' in persistence capable class '"
+ cds.getClassDescriptor().getClassNameOfObject() +
"'");
}
// performance optimization
if(optimized)
{
cds.setCollectionClass(result);
}
}
return result;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-user-help@db.apache.org
Re: Bugs in 1.0.5rc1
Posted by Sascha Broich <sa...@tsa.de>.
Hi Armin
>
> yep, this is the intended behavior. This method (beside user specific
> ManageableCollection implementations) only allow base collection
> classes
> (List, Set,...).
> The mentioned line is similar to:
> Set.class.equals(fieldClass)
Okay, if this is intended.
It was just a stumbling block.
I will look if I can change our code with backward compatibility.
>
> You are right, this should not happen. In the OJB test-suite we do
> several tests using table-per-subclass inheritance and this never
> happens.
> Could you post the mapping for class A1, A0 and the source of the
query
> then I can try to reproduce your issue.
The criteria for this issue was build by using the table column name
'PHB_USRID'.
After I changed this to 'addIsNull("m_iUserId") the query succeeded.
Here is some code:
public static Collection getPublic()
{
Criteria criteria = new Criteria();
criteria.addColumnIsNull("PHB_USRID");
PersistenceBroker broker =
PersistenceBrokerFactory.defaultPersistenceBroker();
QueryByCriteria query = new QueryByCriteria(A1.class, criteria);
query.setDistinct(bDistinct);
return broker.getCollectionByQuery(query);
}
/**
*
* @ojb.class table="A1"
*/
public class A1 {
/**
* @ojb.field column="PHB_ID"
* primarykey="true"
* autoincrement="ojb"
* id="1"
*/
protected Integer m_iId;
/**
* @ojb.field column="PHB_NAME"
* jdbc-type="VARCHAR"
* length="50"
*/
protected String m_sName;
/**
* @ojb.field column="PHB_USRID"
*/
protected Integer m_iUserId;
}
/**
* @ojb.class table="A0"
* include-inherited="false"
* @ojb.field column="PHB_ID"
* primarykey="true"
* name="m_iId"
* jdbc-type="INTEGER"
* @ojb.reference class-ref="A1"
* auto-retrieve="true"
* auto-update="true"
* auto-delete="true"
* foreignkey="m_iId"
*/
public class A0 extends A1
{
/**
* @ojb.field column="PHB_CSV"
* jdbc-type="VARCHAR"
* length="255"
*/
protected String csv;
}
I hope it is enough, because I can't post pure company code.
Regards,
Sascha Broich
---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-user-help@db.apache.org
Re: AW: Bugs in 1.0.5rc1
Posted by Armin Waibel <ar...@apache.org>.
Hi Sascha,
Sascha Broich wrote:
> Hi Armin,
>
>> The method CollectionTypes#getCollectionClass is correctly
> implemented.
>> First this method checks for user specific collection class
>> implementations (implementations of ManageableCollection), then this
>> method try to resolve the collection class implementations for 1:n and
>> m:n collection-fields (of type Collection, List, Set or array types)
> of
>> persistence capable classes.
>>
>> The "real bug" is a typo in the OJB.properties file:
>> replace
>>
> CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.Man
>> ageableSet
>> with
>>
> CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.Man
>> ageableHashSet
>>
>> I will fix this ASAP. Thanks again.
>
> I'm sorry, but the change in the OJB.properties file didn't solve the
> problem.
> I still get the MetadataException "Cannot determine a collection type
> for collection/list/set/array field 'devices' of type 'class
> java.util.HashSet' in persistence capable class 'Abc'"
>
> As I wrote: HashSet.class.isAssignableFrom(java.util.Set.class) will
> ALWAYS fail.
yep, this is the intended behavior. This method (beside user specific
ManageableCollection implementations) only allow base collection classes
(List, Set,...).
The mentioned line is similar to:
Set.class.equals(fieldClass)
> Please remember, the method can only be successful in the direction
> superclass.isAssignableFrom(subclass).
> The only superclass of java.util.Set is java.util.Collection.
> So as long as there is a HashSet instead of a Set used as field the
> check fails, despite the OJB.properties file.
>
> In 1.0.4 this assignable check was against RemovalAwareSet, which is a
> subclass of HashSet.
yep, you are right. But now the collection types are configurable in
OJB.properties file e.g.
CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.ManageableHashSet
So we can't check against an implementation class.
Currently RC1 automatic resolve only base collection types like
Collection, List, Vector and Set.
Two workarounds:
1.) If you change field 'devices' to type 'Set' it should work (with
patched OJB.properties file).
2.) Directly set a collection implementation class in the
collection-descriptor (you can use a shortcut-name for shipped
implementations - see documentation):
<collection-descriptor
name="allArticlesInGroup"
element-class-ref="org.apache.ojb.broker.Article"
collection-class="set"
...
</collection-descriptor>
I have to admit that the current behavior isn't perfect ;-), so I will
improve class CollectionTypes to be backward compatible with 1.0.4:
First check the collection field base type (e.g. subclass of class Set)
and then make sure that the ManageableCollection implemetation class is
a subclass of the field collection type.
<snip>
if (ManageableCollection.class.isAssignableFrom(fieldType))
{
result = fieldType;
}
else if(Set.class.isAssignableFrom(fieldType))
{
result = cds.isMtoNRelation() ? getManyToManySet() : getOneToManySet();
if(!fieldType.isAssignableFrom(result))
{
failed = true;
}
}
...
</snip>
>
>>> Another bug is in the statement creation when a subclass is
> involved.
>>> Note: A0 is the subclass table from A1, PHB_USRID exists only in A1
>>>
>>> 1.0.4 creates something like
>>>
>>> SELECT
>>> A0.PHB_ID ,
>>> A1.PHB_USRID ,
>>> FROM
>>> IPT_PHONEBOOKCSV A0
>>> INNER JOIN IPT_PHONEBOOK A1
>>> ON
>>> A0.PHB_ID = A1.PHB_ID
>>> WHERE
>>> PHB_USRID IS NULL
>>>
>>>
>>>
>>> 1.0.5rc1 creates
>>>
>>> SELECT
>>> A0.PHB_ID ,
>>> A1.PHB_ID ,
>>> A1.PHB_USRID ,
>>> FROM
>>> IPT_PHONEBOOKCSV A0
>>> INNER JOIN IPT_PHONEBOOK A1
>>> ON
>>> A0.PHB_ID = A1.PHB_ID
>>> WHERE
>>> A0.PHB_USRID IS NULL
>>>
>>>
>>> The WHERE criteria assignment to A0 the statement produces an
>> exception
>>> for the nonexisting column A0.PHB_USRID.
>>>
>>> Note, that in the SELECT of 1.0.5rc1 is also A1.PHB_ID, which is not
>> in
>>> the statement of 1.0.4.
>> If PHB_USRID only exists in A1 then A1 is a subclass of (super) class
>> A0. In this case it's not allowed to build a query with target class
> A0
>> and fields only existing in A1.
>> Is this the case? If not, please post some more details (java pseudo
>> code, class mapping).
>
> class A1
> {
> PHB_ID;
> PHB_USRID;
> PHB_NAME;
> }
>
> class A0 extends A1
> {
> PHB_CSV;
> }
>
> And the tables are
> Table A1
> PHB_ID
> PHB_USRID
> PHB_NAME
>
> Table A0
> PHB_ID
> PHB_CSV
>
> Because of the inheritance there shouldn't be a need for the column
> PHB_USRID in the table for the derived class A0.
>
> The correct statement should be
>
> SELECT
> A0.PHB_ID ,
> A0.PHB_CSV ,
> A1.PHB_USRID,
> A1.PHB_NAME
> FROM
> A0 INNER JOIN A1 ON A0.PHB_ID = A1.PHB_ID
> WHERE
> A1.PHB_USRID IS NULL
>
> I'm not sure if the 1.0.4 resolved this correct but it didn't add the
> alias to the where clause so the database assigned the correct column by
> itself.
You are right, this should not happen. In the OJB test-suite we do
several tests using table-per-subclass inheritance and this never happens.
Could you post the mapping for class A1, A0 and the source of the query
then I can try to reproduce your issue.
regards,
Armin
>
>
> Regards,
> Sascha Broich
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-user-help@db.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-user-help@db.apache.org
AW: Bugs in 1.0.5rc1
Posted by Sascha Broich <sa...@tsa.de>.
Hi Armin,
> The method CollectionTypes#getCollectionClass is correctly
implemented.
> First this method checks for user specific collection class
> implementations (implementations of ManageableCollection), then this
> method try to resolve the collection class implementations for 1:n and
> m:n collection-fields (of type Collection, List, Set or array types)
of
> persistence capable classes.
>
> The "real bug" is a typo in the OJB.properties file:
> replace
>
CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.Man
> ageableSet
> with
>
CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.Man
> ageableHashSet
>
> I will fix this ASAP. Thanks again.
I'm sorry, but the change in the OJB.properties file didn't solve the
problem.
I still get the MetadataException "Cannot determine a collection type
for collection/list/set/array field 'devices' of type 'class
java.util.HashSet' in persistence capable class 'Abc'"
As I wrote: HashSet.class.isAssignableFrom(java.util.Set.class) will
ALWAYS fail.
Please remember, the method can only be successful in the direction
superclass.isAssignableFrom(subclass).
The only superclass of java.util.Set is java.util.Collection.
So as long as there is a HashSet instead of a Set used as field the
check fails, despite the OJB.properties file.
In 1.0.4 this assignable check was against RemovalAwareSet, which is a
subclass of HashSet.
> > Another bug is in the statement creation when a subclass is
involved.
> > Note: A0 is the subclass table from A1, PHB_USRID exists only in A1
> >
> > 1.0.4 creates something like
> >
> > SELECT
> > A0.PHB_ID ,
> > A1.PHB_USRID ,
> > FROM
> > IPT_PHONEBOOKCSV A0
> > INNER JOIN IPT_PHONEBOOK A1
> > ON
> > A0.PHB_ID = A1.PHB_ID
> > WHERE
> > PHB_USRID IS NULL
> >
> >
> >
> > 1.0.5rc1 creates
> >
> > SELECT
> > A0.PHB_ID ,
> > A1.PHB_ID ,
> > A1.PHB_USRID ,
> > FROM
> > IPT_PHONEBOOKCSV A0
> > INNER JOIN IPT_PHONEBOOK A1
> > ON
> > A0.PHB_ID = A1.PHB_ID
> > WHERE
> > A0.PHB_USRID IS NULL
> >
> >
> > The WHERE criteria assignment to A0 the statement produces an
> exception
> > for the nonexisting column A0.PHB_USRID.
> >
> > Note, that in the SELECT of 1.0.5rc1 is also A1.PHB_ID, which is not
> in
> > the statement of 1.0.4.
>
> If PHB_USRID only exists in A1 then A1 is a subclass of (super) class
> A0. In this case it's not allowed to build a query with target class
A0
> and fields only existing in A1.
> Is this the case? If not, please post some more details (java pseudo
> code, class mapping).
class A1
{
PHB_ID;
PHB_USRID;
PHB_NAME;
}
class A0 extends A1
{
PHB_CSV;
}
And the tables are
Table A1
PHB_ID
PHB_USRID
PHB_NAME
Table A0
PHB_ID
PHB_CSV
Because of the inheritance there shouldn't be a need for the column
PHB_USRID in the table for the derived class A0.
The correct statement should be
SELECT
A0.PHB_ID ,
A0.PHB_CSV ,
A1.PHB_USRID,
A1.PHB_NAME
FROM
A0 INNER JOIN A1 ON A0.PHB_ID = A1.PHB_ID
WHERE
A1.PHB_USRID IS NULL
I'm not sure if the 1.0.4 resolved this correct but it didn't add the
alias to the where clause so the database assigned the correct column by
itself.
Regards,
Sascha Broich
---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-user-help@db.apache.org
Re: Bugs in 1.0.5rc1
Posted by Armin Waibel <ar...@apache.org>.
Hi Sascha,
thank you very much for testing the RC1!
Sascha Broich wrote:
> Hello,
>
> I found a bug in
> org.apache.ojb.broker.core.CollectionTypes#getCollectionClass(Collection
> Descriptor).
> The "isAssignableFrom" check has to be inverted.
> For instance:
> HashSet.class.isAssignableFrom(Set.class) yields false and a
> MetadataException is thrown.
> Inverted to Set.class.isAssignableFrom(HashSet.class) yields true and
> the correct branch gets entered.
The method CollectionTypes#getCollectionClass is correctly implemented.
First this method checks for user specific collection class
implementations (implementations of ManageableCollection), then this
method try to resolve the collection class implementations for 1:n and
m:n collection-fields (of type Collection, List, Set or array types) of
persistence capable classes.
The "real bug" is a typo in the OJB.properties file:
replace
CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.ManageableSet
with
CollectionTypes.OneToManySet=org.apache.ojb.broker.util.collections.ManageableHashSet
I will fix this ASAP. Thanks again.
>
>
> Another bug is in the statement creation when a subclass is involved.
> Note: A0 is the subclass table from A1, PHB_USRID exists only in A1
>
> 1.0.4 creates something like
>
> SELECT
> A0.PHB_ID ,
> A1.PHB_USRID ,
> FROM
> IPT_PHONEBOOKCSV A0
> INNER JOIN IPT_PHONEBOOK A1
> ON
> A0.PHB_ID = A1.PHB_ID
> WHERE
> PHB_USRID IS NULL
>
>
>
> 1.0.5rc1 creates
>
> SELECT
> A0.PHB_ID ,
> A1.PHB_ID ,
> A1.PHB_USRID ,
> FROM
> IPT_PHONEBOOKCSV A0
> INNER JOIN IPT_PHONEBOOK A1
> ON
> A0.PHB_ID = A1.PHB_ID
> WHERE
> A0.PHB_USRID IS NULL
>
>
> The WHERE criteria assignment to A0 the statement produces an exception
> for the nonexisting column A0.PHB_USRID.
>
> Note, that in the SELECT of 1.0.5rc1 is also A1.PHB_ID, which is not in
> the statement of 1.0.4.
If PHB_USRID only exists in A1 then A1 is a subclass of (super) class
A0. In this case it's not allowed to build a query with target class A0
and fields only existing in A1.
Is this the case? If not, please post some more details (java pseudo
code, class mapping).
regards,
Armin
>
>
> Regards,
> Sascha Broich
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-user-help@db.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-user-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-user-help@db.apache.org