gemire / daofusion

Automatically exported from code.google.com/p/daofusion
0 stars 1 forks source link

H2: NestedPropertyCriteria created by NestedPropertyCriteriaBasedConverter fails within AbstractHibernateEntityDao#count() #1

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Assume a scenario with CriteriaTransferObject containing paging information
along with FilterAndSortCriteria instances employing both filtering and
sorting capabilities.

When NestedPropertyCriteriaBasedConverter converts CriteriaTransferObject
instance into NestedPropertyCriteria, this NestedPropertyCriteria contains
all of the above (paging, filtering, sorting).

Passing this NestedPropertyCriteria instance into
AbstractHibernateEntityDao#count() results in SQL exception using H2
database. Experiments have shown that Oracle 10g handles this case without
problems.

Root cause of this issue is most probably that
NestedPropertyCriteria#apply() performs changes to target Hibernate
Criteria instance which are not appropriate for semantics of count() -
paging and sorting.

DAO Fusion version: 1.0.1
Database: H2 1.0.79

Proposed solution: NestedPropertyCriteriaBasedConverter must distinguish
two typical cases for which specific NestedPropertyCriteria instance is
required:
- query (subset of entities for given criteria)
- count (filtering criteria only)

Two methods could be added to NestedPropertyCriteriaBasedConverter:
- convertForQuery()
- convertForCount()

As a result, NestedPropertyMapping#apply() will be replaced by similar
methods for two cases presented above.

Original issue reported on code.google.com by igor.mih...@gmail.com on 27 Feb 2009 at 3:34

GoogleCodeExporter commented 9 years ago
In case of PersistentEntityCriteria and its default implementation
(NestedPropertyCriteria), it is the responsibility of the user to set it up 
properly
according to the specific use case in conjunction with core DAO methods (e.g. it
doesn't make sense to add paging or sort information when performing entity 
count).
This same principle applies to Hibernate Criteria API as well.

This issue covers the case when NestedPropertyCriteria is constructed 
automatically
from CriteriaTransferObject - in this case, the converter cannot know in which
context (query/count) the resulting NestedPropertyCriteria will be used.

Proposed solution seems adequate since it targets CTO classes without affecting 
core
DAO functionality.

This issue will be fixed in DAO Fusion release 1.0.2.

Original comment by Vojtech....@gmail.com on 2 Mar 2009 at 9:51

GoogleCodeExporter commented 9 years ago
I took DAOFusion for a spin and I came across another problem (but related). If 
you
try to invoke AbstractHibernateEntityDao#count() on PersistentEntityCriteria 
which
defines only paging parameters, H2 will return empty list and
java.lang.IndexOutOfBoundsException will be thrown from
BaseHibernateDataAccessor#rowCount method. See attached SVN patch for details of
JUnit test used.

DAOFusion and Hibernate pass all parameters correctly, you can check SQL 
queries and
its parameters when enabling console logging on H2. In spite of that, it seams 
that
H2 cannot process count queries with paging parameters properly (but you know 
that
already:-).

The best solution for DAOFusion could be to ignore (silently or with warnings) 
paging
and sorting criteria when count query is issued. Anyway there is not much sense 
in
making result set ordered when querying DB for count.

Original comment by michal.j...@gmail.com on 6 Mar 2009 at 2:34

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Michal and welcome to the project :-)

The problem you pointed out was actually the root cause for this issue and is 
the
result of combining paging information with projections (rowCount).

What happens in this situation is:
- you create NestedPropertyCriteria instance with paging information and pass 
it to
AbstractHibernateEntityDao#count()
- Hibernate Criteria instance is created via 
BaseHibernateDataAccessor#getCriteria()
from NestedPropertyCriteria with firstResult/maxResults being set
- Hibernate Criteria instance is passed to BaseHibernateDataAccessor#rowCount() 
which
sets the "rowCount" projection and tries to get the first (and only) result 
list value

Resulting SQL looks like this:
SELECT COUNT(*) FROM ... [paging constraints go here]

So on one hand, we say "select the row count as a single result value from this
query" and on the other hand, we say "but limit result set from X to Y". Of 
course,
the "other hand" statement doesn't make any sense and from my point of view H2
behavior is correct in this case.

We get the java.lang.IndexOutOfBoundsException because we have manipulated 
paging
information and got an empty list (from which we try to fetch the first value).

I think the proposed solution is adequate since it affects CTO classes only - 
CTO
converter is the only place where NestedPropertyCriteria is created 
automatically and
CTO converter cannot know in which context it will be used. On the other hand, 
when
using NestedPropertyCriteria "manually", the user is responsible for setting its
properties properly.

Original comment by Vojtech....@gmail.com on 6 Mar 2009 at 9:56

GoogleCodeExporter commented 9 years ago
Thanks Michal for the test patch, I will update AbstractHibernateEntityDaoTest 
to
include a test method regarding count() with filter constraints.

I have extended the BaseHibernateDataAccessor#rowCount() implementation as well 
- now
a check is made whether the rowCount projection results in a single Ingeger 
value, if
not, a warning is logged and zero is returned.

Original comment by Vojtech....@gmail.com on 6 Mar 2009 at 10:27

GoogleCodeExporter commented 9 years ago
Update: we formalized our release policy and since this issue causes slight API
changes, it will be fixed in DAO Fusion 1.1.0.

Original comment by Vojtech....@gmail.com on 9 Mar 2009 at 1:06

GoogleCodeExporter commented 9 years ago
Issue is fixed in the trunk and will be part of the 1.1.0 release.

The final solution is based on Igor's CriteriaTransferObject wrapper proposal. 
The
CriteriaTransferObjectCountWrapper class represents a server-side CTO wrapper 
for
entity instance count purposes. It takes the original CTO instance as its 
constructor
argument and provides a method called "wrap" which creates new CTO instance. 
This new
CTO instance delegates most of its methods to the wrapped (original) CTO with 
the
exception of paging and sort constraints and methods that modify internal state 
of
the transfer object.

Typical usage pattern is like this:

PersistentEntityCriteria criteriaForCount = converter.convert(
    new CriteriaTransferObjectCountWrapper(transferObject).wrap(),
    myMappingGroup);

// now you can pass "criteriaForCount" into AbstractHibernateEntityDao#count() 
methods

CriteriaTransferObjectCountWrapperTest was added as well (this ensures that the
resulting CTO instance has its paging and sort constraints suppressed properly).

Some comments were added to the Javadoc as well regarding PersistentEntityDao /
AbstractHibernateEntityDao / BaseHibernateDataAccessor just to make things 
about this
"rowCount" projection stuff more clear to the end-user.

Original comment by Vojtech....@gmail.com on 12 Mar 2009 at 3:00

GoogleCodeExporter commented 9 years ago

Original comment by Vojtech....@gmail.com on 12 Mar 2009 at 3:05

GoogleCodeExporter commented 9 years ago
CriteriaTransferObjectCountWrapperTest unit test refactoring was done and 
submitted
for code-review: http://codereview.appspot.com/28044

Original comment by igor.mih...@gmail.com on 14 Mar 2009 at 9:08