6:16 p.m.
When you form a Hibernate Disjunction criteria, but don't add any
clauses to it, you get a criteria that matches everything.
Generally, when you perform a reduction over an empty list, the
expected result is the identity element for the reduction. For
example, the sum of the empty list is 0, the product of the empty list
is 1, the conjunction of the empty list is true, and the disjunction
of the empty list is false.
Hibernate uses "1=1" as the SQL for both Conjunction and Disjunction
criteria if there is an empty list of criteria.
Seems to me that this is a bug, although Disjunction isn't documented,
so who knows what it is "supposed" to do.
Bill Pugh
11:30 p.m.
New subject: Hibernate Disjunction Criteria broken: returns true for empty list of criteria
William Pugh wrote:
Generally, when you perform a reduction over an empty list, the
expected result is the identity element for the reduction. For
example, the sum of the empty list is 0, the product of the empty list
is 1, the conjunction of the empty list is true, and the disjunction
of the empty list is false.
Hibernate uses "1=1" as the SQL for both Conjunction and Disjunction
criteria if there is an empty list of criteria.
Seems to me that this is a bug, although Disjunction isn't documented,
so who knows what it is "supposed" to do.
Yes the whole point of all WHERE clauses is to filter/reduce the
available data-set. I see this point of view but...
In the interest of capturing the most bugs during development by
incorrect usage (incorrect understanding) of the API, which is the
easiest thing to spot ?
A dataset returning more data than expected (everything) ?
A dataset returning less data than expected (nothing) ?
My preference goes to returning an extreme state of what you are
expecting to achieve with the operator.
OR is generally used to increase the size of the returned data-set. So
to capture the most bugs we return more data than expected "OR 1=1".
AND is generally used to decrease the size of the returned data-set. So
to capture the most bugs we return less data than expected "AND 1=0".
Maybe there is also a case for warning the user when an empty
Conjunction() and Disjuction() is being used with an option to turn this
warning off. I really do think that should be considered a problematic
programming error.
From my point of view there is an argument for having either another
pair of classes or a new API method (my prefered) :
ConjunctionAnulIfEmpty()
DisjunctionAnulIfEmpty()
or with new API method:
Conjunction().setAnulMode(ANUL_IF_EMPTY);
Conjunction().setAnulMode(TRUE_IF_EMPTY);
Conjunction().setAnulMode(FALSE_IF_EMPTY);
Conjunction().setAnulMode(COMPAT_IF_EMPTY); // Does whatever the
previous version of HIB did, is also the default
Disjunction().setAnulMode(ANUL_IF_EMPTY);
Disjunction().setAnulMode(TRUE_IF_EMPTY);
Disjunction().setAnulMode(FALSE_IF_EMPTY);
Disjunction().setAnulMode(COMPAT_IF_EMPTY); // Does whatever the
previous version of HIB did, is also the default
If setAnulMode() is used in anyway then no warning is ever emitted for
that statement.
Maybe there should also be API method:
disjunction().isEmpty()
conjunction().isEmpty()
as well as better anul handling. This is because you have to maintain
that boolean state yourself in code which again leads to more ugly
looking code around Critera API use.
This is for Criteria API use cases where you don't want to create a
temporary Junction, work out if you need it and the attach it to the
query if you did need it. You just want Criteria API to ignore it if
its empty but be able to always attach it to its parent Expression.
Which version if hibernate are you reporting this on ? There was a
change at sometime to something in this realm in the release notes.
My $0.02's,
Darryl
12:54 p.m.
Yes the whole point of all WHERE clauses is to filter/reduce the available
data-set. I see this point of view but...
No, the point of a where clause it to give you the data you want.
A query that gives me no results back is far more useful and common
than a query that materializes my entire database in memory.
I know of a real application that had a serious problem because this
bug in hibernate caused the system to crash because it swamped memory.
The standard logic for handling an disjunction is:
if any clause is true, return true
otherwise, return false
Having the logic the way it is currently implemented introduces all
sorts of logical discontinuities in Hibernate.
For example, Or( X, Or(Y | Z)) != Or( Or(X, Y) | Z), using | to
represent list concatenation.
Frankly, the claim that Or of the empty list should return true isn't
credible. To be blunt, I don't want to use any software written by
someone who believes that.
AND is generally used to decrease the size of the returned data-set.
So to
capture the most bugs we return less data than expected "AND 1=0".
Also bogus. The standard logic for handling a conjunction is:
if any clause is false return false
otherwise, return true
Bill Pugh
My personal opinion, not that of any of my employers
2:21 p.m.
Ahead of my claim is that an empty Junction in Criteria API is really a
programming concern (possibly a programming/usage error).
I understand your point of view of the way you might expect it to work
if you were creating an SQL query builder and wanted to deal with the
empty junction situation gracefully.
My view point comes from an empty Junction is a programming error,
therefore how do we detect this error soonest during development, we do
that by causing the most havoc by working the opposite way, therefore
the application should never get through testing. Now if an empty
junction is not an error then I'd agree with your logic when using
Critera API as an SQL query builder since that is the natural way.
I know of situations where the Criteria API application code would look
elegant and cleaner if there was such an AnnulMode setting to Junctions
to deal with empty junctions.
Maybe an annul mode of AnnulMode.NATURAL_IF_EMPTY would work the way you
describe that would make sense to me. It stays in the query but flips
the way you describe.
We can always argue over what everyone thinks the default should be (I
couldn't not care less what the default is, if only I can set AnnulMode)
I just want good API design that will cope with common situations I have
experienced.
As for the you know of a real app that crashed etc... thats your testing
departments and QAs problem.
As for you not wishing to use such and such software, well for a start I
am in no way a HIB committer, I have experience with just this problem
with Junctions and Criteria API myself.
Line 76 I guess:
http://fisheye.jboss.com/browse/~br=root%3A/Hibernate/core/trunk/core/src...
Darryl
4:27 p.m.
Hello,
When you form a Hibernate Disjunction criteria, but don't add
any
clauses to it, you get a criteria that matches everything.
Generally, when you perform a reduction over an empty list, the
expected result is the identity element for the reduction. For
example, the sum of the empty list is 0, the product of the empty list
is 1, the conjunction of the empty list is true, and the disjunction
of the empty list is false.
You're right of course, Hibernate shouldn't
stray away from ordinary
logic :) I would qualify it as a bug.
(that's only my opinion of course)
In fact, I have to fix this in Envers too, as after changing the
versions-query implementation not to use criteria, this will cause a
run-time exception - so thanks :).
--
Adam
6:03 a.m.
New subject: Hibernate Disjunction Criteria broken: returns true for empty list of criteria
William Pugh wrote:
When you form a Hibernate Disjunction criteria, but don't add
any
clauses to it, you get a criteria that matches everything.
Here is my crack at it:
http://opensource.atlassian.com/projects/hibernate/browse/HHH-3583
I still maintain that adding empty Disjunction() in Critera API
expression is a programming error. Empty Conjunction() only work by
chance and this is because of the implicit enclosing Conjunction() due
to the use of the add() method in the API like:
critera.add(Expression.someexpr()).add(Expression.someexpr());
To prove this point (that historically it's a programming error) try
using a mixture of empty Conjuction() (or empty Disjunction())
expressions inside other each other and see how soon you become unstuck.
resultList = s.createCriteria(Student.class)
.add( Restrictions.disjunction()
.add( Property.forName("studentNumber").gt( new Long(666) ) )
.add( Property.forName("studentNumber").lt( new Long(102) ) )
.add( Restrictions.disjunction()
.add( Restrictions.disjunction() )
.add( Restrictions.conjunction()
.add( Restrictions.conjunction() )
.add( Restrictions.conjunction() )
)
)
)
.addOrder( Order.asc( "studentNumber" ) )
.list();
Hopefully the patch produces a good/better API.
Darryl
5825
days inactive
5829
days old
5 comments
3 participants
participants (3)
-
Adam Warski -
Darryl Miles -
William Pugh