hibernate-devel — development

Re: [Hibernate-devel] Queries on objects with composite keys...

[Jon L. <jon...@xe...>]

Hi...

I just wanted to give you a quick update...  I have the query by composite
keys working now.  I will do some more testing throughout the weekend and
early next week to make sure that everything else still works.  If it
everything checks out with no problems, I'll post the patches the middle of
next week.

Cheers,
Jon...

----- Original Message -----
From: <Gavin_King/Cirrus%CI...@ci...>
To: <hib...@li...>
Sent: Wednesday, May 15, 2002 2:42 PM
Subject: Re: [Hibernate-devel] Queries on objects with composite keys...


>
> Cool. Looking forward to it.
>
> Peace
>
> >> It might take a few days to implement this since I'm in the middle of
> another project, but since I need this in quite a few places I am
motivated
> to implement this.
>
>
> _______________________________________________________________
>
> Have big pipes? SourceForge.net is looking for download mirrors. We supply
> the hardware. You get the recognition. Email Us: ban...@so...
> _______________________________________________
> Hibernate-devel mailing list
> Hib...@li...
> https://lists.sourceforge.net/lists/listinfo/hibernate-devel
>

RE: [Hibernate-devel] Simple transaction management

[Anton v. S. <an...@ap...>]

> hhmmmmmmm .... damn this is all coming back to haunt .....

Sorry to rehash - I hadn't seen the previous discussion.  I'll check out the
archives to catch up, although I haven't yet done so in the response below.

> Lets state the problem carefully:
>
> What API allows the user to END a Session and
>
> (1) is easy to use and understand for new users,
> (2) has methods with clear semantics,
> (3) supports the various different possible styles of connection +
> transaction management eg. application server datasource + JTA,
> DriverManager + Connection.commit(), long transactions,
> (4) comforms to the philosophy that Hibernate is a thin wrapper
> around JDBC that exposes all underlying JDBC functionality to
> the application. (A wrapper with "windows"?)

I agree with all of that.  I'm proposing two additional requirements, one of
which I stated explicitly and one of which I didn't (but should have).  The
one I stated explicitly was:

> application code could switch the underlying transaction
> handling mechanism without changing the client code

Aside from the convenience of this, I think that the current design with
session.connection().xxx encourages users to depend on direct access to the
Connection, and doesn't provide an alternative.  In a sense, this leads
users down the wrong path - Hibernate abstracts all sorts of things but then
just exposes the connection and encourages its use.  I'm all for the idea of
"a wrapper with windows", but I'm also suggesting that it shouldn't be
necessary to use those windows.  It should be an option for those who want
it.

My second requirement comes from never having been a fan of the JDBC
transaction API, and its reliance on toggling autoCommit to begin and end
transactions.  This is unintuitive, doesn't "say what it means", and can
have bad consequences in the case of mistakes.  For example, forgetting to
setAutoCommit(true) at the end of a transaction results in all subsequent
requests on that connection being lumped into a single transaction, with
potentially horrible effects on concurrency.

I'm suggesting that a very thin wrapper here would be beneficial in general,
and I think it's appropriate for Hibernate to offer this as an optional
alternative, to encourage sensible transaction usage that reads well in the
application code, guards against certain kinds of errors, and corrects the
egregious mistakes that Sun made. ;)

If the above two additional requirements are met, I'm not as concerned about
the exact form this API takes.  OTOH, if everyone thinks I've been smoking
the $2 crack, I'll stick to using my own wrapper to get the functionality I
want.  What prompted me to start this thread was the questions that have
arisen about what Sessions are, and the attempt to clarify them in
documentation.

I think part of the problem is that the relationship between a Session and a
transaction isn't as clear as it might be, and this is at least partly
because the separation between the two is currently implemented by saying
"here's the connection, you're on your own with transactions".  I imagine
this is likely to encourage the tendency of users to think of a session as
being like a connection, which you can use repeatedly for multiple
transactions.

I think that making it unnecessary to directly access the connection would
be of general benefit to Hibernate users, and as a side benefit, may be less
likely to result in confusion, although obviously that's just a guess.  It
would still be necessary to make it clear, for example, that multiple
transactions shouldn't be executed on a single session.

I suppose commitTransaction() and rollbackTransaction() could also simply
close the connection - more on this below.

> When we had this discussion earlier, Brad Clow commented that one
> thing that was so confusing was that we were overloading the word
> "commit" to mean something other than committing a transaction.
> I agreed at the time, and still agree. If we do something like
> you are suggesting, we should not use the words "commit", "begin"
> "rollback" or "transaction" anywhere on the Session interface.

That was why I suggested method names like "commitTransaction".  My intent
was to clearly delineate between operations on the Hibernate Session, and
the conceptual/database transaction.

I was also trying to stick more closely to the current API's approach, and
basically suggesting that session.connection().commit() could simply be
(optionally) replaced by session.commitTransaction(), which would buy some
additional abstraction and thus flexibility.  My suggestion actually doesn't
change the current API all that much, and of course the existing API could
still be used as is, i.e. it's a backwards-compatible addition.

> public interface Session {
>   .....
>   public void end() throws ....;     //commit, close
>   public void cancel() throws ....;  //rollback, close
>   public void pause() throws ....;   //commit, disconnect
>   public void resume() throws .....; //reconnect
> }

This would satisfy my concerns above in terms of its functionality.  I have
two further observations:

1.  The begin/commit/rollback idiom is pretty well known and understood.
Essentially making up new words for these doesn't necessarily help new
users.

2.  I do see some benefit to separating the transaction functionality from
the session.  For example (and perhaps I just need more info here): what
about the case where a session will be "read-only", so that an explicit
overall transaction isn't needed, i.e. equivalent to JDBC
setAutoCommit(true)?  If an overall transaction is implicitly created by
Hibernate, it could reduce concurrency unnecessarily.  This was one reason
why I thought it might be a good idea to separate out the transaction calls
from the session open/close.  A readonly session could simply omit the
transaction calls, and perhaps include an initial call to suspendFlushes(),
for example.  I think this might be simpler than adding features to the
above API to support this case.

> I'm also not keen on changing the cirrus.hibernate.Session interface again
> at this stage. It might be preferable to develop this API (ie. the new
> Session and SessionFactory interfaces) in a seperate package. Say,
> cirrus.hibernate.4dummies or something. ;-> The new functionality could be
> implemented either on the existing cirrus.hibernate.impl classes or as
> wrapper classes that delegate to cirrus.hibernate.SessionFactory and
> cirrus.hibernate.Session.

I'm sensitive to this concern.  Again, that was a reason I proposed adding
xxxTransaction() to Session, since it wouldn't break existing code, and it
could be an optional mechanism.  If the other approach is decided on, making
a Session the same as a transaction, perhaps an alternative to Session would
be needed.

I'm being very outspoken for a brand new Hibernate user working on his first
application with it.  All Gavin's fault for encouraging me. :)  If I'm
missing something or am wrong about this, please apply cluestick to my
noggin with appropriate force.  But even if my conclusions aren't palatable,
I think at least the concern about wrapping connections has validity, and
I'd be interested in figuring out (and working on) a sufficiently general
way to address that.

Anton

Re: [Hibernate-devel] Simple transaction management

[Gavin_King/Cirrus%<CI...@ci...>]

> I know that commit() and cancel() were recently deprecated, but
> I'm thinking it might be a good idea to add some methods for a
> slightly more general transaction handling mechanism to the Session
> class.

hhmmmmmmm .... damn this is all coming back to haunt .....

Lets state the problem carefully:

What API allows the user to END a Session and

(1) is easy to use and understand for new users,
(2) has methods with clear semantics,
(3) supports the various different possible styles of connection +
transaction management eg. application server datasource + JTA,
DriverManager + Connection.commit(), long transactions,
(4) comforms to the philosophy that Hibernate is a thin wrapper around JDBC
that exposes all underlying JDBC functionality to the application. (A
wrapper with "windows"?)

The old approach with Session.commit() + Session.cancel() tried to be a
great solution to (1) but because it miserably failed at (2) ended up being
very confusing for _all_ users.

The new API is a much better solution in terms of (2), (3) and (4) -
particularly (2) - but still requires a fairly lengthy explanation about
how to actually end a Session - something that should, on the face of it,
be so damn simple!!

> What I have in mind is that Session would support at least three
> additional methods, something like beginTransaction,
> commitTransaction, and rollbackTransaction.  These would delegate
> to an object which implements a "SimpleTransactionManager" interface
> with those methods.

When we had this discussion earlier, Brad Clow commented that one
thing that was so confusing was that we were overloading the word
"commit" to mean something other than committing a transaction.
I agreed at the time, and still agree. If we do something like
you are suggesting, we should not use the words "commit", "begin"
"rollback" or "transaction" anywhere on the Session interface. I
would suggest something along the lines of:

public interface TransactionStrategy {
  public Connection begin() throws ....;
  // fetch a connection
  public void commit(Connection connection) throws ...;
  // commit, close connection
  public void rollback(Connection connection) throws ....;
  // rollback, close connection
}

public interface Session {
  .....
  public void end() throws ....;     //commit, close
  public void cancel() throws ....;  //rollback, close
  public void pause() throws ....;   //commit, disconnect
  public void resume() throws .....; //reconnect
}

Then client code could look like:

Session s = factory.openSession(sessionLifecycle);
try {
  //do some work
  ....
  s.flush();
}
catch (Exception e)
  s.cancel();
  throw e;
}
s.end();

Session.pause() and Session.resume() would be used to disconnect
and reconnect.

> Hibernate could supply a couple of really
> simple implementations of this interface, e.g. one which just maps
> to JDBC calls, and

The implementations I can think of would be:

DriverManagerTransactionStrategy
JTADatasourceTransactionStrategy
NonJTADatasourceTransactionStrategy

> perhaps one which maps to "BEGIN TRAN", "COMMIT
> TRAN", "ROLLBACK TRAN"  statements as appropriate for the SQL
> dialect.

JDBC already does this mapping, by providing Connection.commit()
and Connection.rollback() ...

> application code could switch the underlying transaction
> handling mechanism without changing the client code

This is a 'new' requirement that is not implemented by the current API. It
seems a worthwhile addition....

> IMO, the transaction-handling idiom would become a bit neater
> and better abstracted:
>
>    Session sess = factory.openSession();
>    try {
>             sess.beginTransaction();
>               //do some work
>             ...
>             sess.commitTransaction();
>    }
>    catch (Exception e) {
>             sess.rollbackTransaction();
>             throw e;
>    }
>    finally {
>               sess.close();
>    }

I need more opinions upon which API is better. flush(), end(), cancel() OR
commitTransaction(), rollbackTransaction(), close()

I'm also not keen on changing the cirrus.hibernate.Session interface again
at this stage. It might be preferable to develop this API (ie. the new
Session and SessionFactory interfaces) in a seperate package. Say,
cirrus.hibernate.4dummies or something. ;-> The new functionality could be
implemented either on the existing cirrus.hibernate.impl classes or as
wrapper classes that delegate to cirrus.hibernate.SessionFactory and
cirrus.hibernate.Session.

After all this thinking, I'm still not 100% convinced we should go down
this path..... (or not yet, anyway). If there is a general clamor of
agreement that this is a much better solution, I guess we should take it.
However, this may just be increasing the complexity of our API to fix a
problem (hard for new users to understand) that would be just as easily
fixed by better documentation.

We have so far managed to cram a large amount of functionality into a very,
very narrow API. This is a Good Thing. Adding new APIs now might not be....

undecided....

Gavin

[Hibernate-devel] Simple transaction management

[Anton v. S. <an...@ap...>]

I know that commit() and cancel() were recently deprecated, but I'm thinking
it might be a good idea to add some methods for a slightly more general
transaction handling mechanism to the Session class.

What I have in mind is that Session would support at least three additional
methods, something like beginTransaction, commitTransaction, and
rollbackTransaction.  These would delegate to an object which implements a
"SimpleTransactionManager" interface with those methods.  Hibernate could
supply a couple of really simple implementations of this interface, e.g. one
which just maps to JDBC calls, and perhaps one which maps to "BEGIN TRAN",
"COMMIT TRAN", "ROLLBACK TRAN" statements as appropriate for the SQL
dialect.  The user could tell the SessionFactory what transaction mechanism
is required, with the option of supplying alternative implementations.  This
could mean that application code could switch the underlying transaction
handling mechanism without changing the client code, and could fairly easily
support things like nested transactions.

I realize this might be more or less irrelevant in some application server
environments, but I think it could be cleaner for simpler applications, and
would eliminate the dependency on direct Connection calls via
session.connection().  Of course, direct use of the Connection would still
be possible, so this wouldn't impose or enforce anything, or break backward
compatibility.

IMO, the transaction-handling idiom would become a bit neater and better
abstracted:

	Session sess = factory.openSession();
	try {
		sess.beginTransaction();
      	//do some work
	      ...
	      sess.commitTransaction();
	}
	catch (Exception e) {
      	sess.rollbackTransaction();
	      throw e;
	}
	finally {
      	sess.close();
	}

I already do this via a Session wrapper of my own, but I'd be happy to
integrate something like this into Hibernate if it's considered worthwhile.
Any feedback is appreciated.

Anton

[Hibernate-devel] performance (please read)

[Gavin_King/Cirrus%<CI...@ci...>]

I have just been doing some new performance tests. I am very pleased with
the results. Hibernate's performance is still very competitive with
handcoded JDBC in my test environment.

Well, with a single caveat (the point of this post):

The "atomic Session" feature is a total performance killer. I should have
realised this earlier but I was always performance testing using
hibernate.atomic_session=false. Atomic sessions allow the application to
continue using a session, even when some kind of exception is thrown from
the persistence layer. I can't think of a really good reason why
applications whould need this feature.

Atomic sessions are currently ENABLED BY DEFAULT. Everyone who wants to see
performance potentially double should DISABLE atomic sessions by setting
the system property (not a SessionFactory property):

hibernate.atomic_session=false

I will rip out this feature in the next version of hibernate, since the
performance cost is ridiculous for a feature of such debatable value.

Gavin

Re: [Hibernate-devel] Queries on objects with composite keys...

[Gavin_King/Cirrus%<CI...@ci...>]

Cool. Looking forward to it.

Peace

>> It might take a few days to implement this since I'm in the middle of
another project, but since I need this in quite a few places I am motivated
to implement this.

Re: [Hibernate-devel] Queries on objects with composite keys...

[Jon L. <jon...@xe...>]

I had the same idea for the syntax, but I wanted to make sure you agreed
with that before I dove in and started working on it.

As a quick hack just to get my app up and running I patched Hibernate to
allow me to queries like:

from foo in class eg.Foo where foo.subid = 69

However I didn't feel like this was an accurate representation of the object
relationships so I now I'd like to do it right.

It might take a few days to implement this since I'm in the middle of
another project, but since I need this in quite a few places I am motivated
to implement this.

Cheers,
Jon...

----- Original Message -----
From: <Gavin_King/Cirrus%CI...@ci...>
To: "Jon Lipsky" <jon...@xe...>
Cc: <hib...@li...>
Sent: Wednesday, May 15, 2002 11:46 AM
Subject: Re: [Hibernate-devel] Queries on objects with composite keys...


>
>
> >> I am correct in my findings that you cannot currently do queries based
> on composite keys where you only want to select using only 1 of the
> composite key properties?  The only way currently supported is to do a
> "find" for the object using the composite key object, right?
>
> Yeah, thats about right. Nor can you frame queries that would require a
> table join on the composite key.
>
> >> I ended up patching Hibernate so you can do a select based on the
> composite key, however I think the way I did it is only valid as a
> temporary solution.  I would be willing to work on adding complete query
> support for composite keys if anyone else thinks there is demand for this.
>
> Sounds great! Any work you could do in this area is _much_
> appreciated!Exactly what are you proposing? I'm thinking a syntax
something
> like:
>
> from foo in class eg.Foo where foo.id.subid = 69
>
> where id is the composite identifier of eg.Foo.
>
> I apologise for all the hand-written parsing code for the query language.
I
> knew zero about lexing + parsing at the time so I ended up doing by hand
> what I should have used ANTLR for. It was my intention to reimplement the
> query language using an antlr grammar I wrote later. (Yeah, I educated
> myself since.) However, since this is the very first feature request to do
> with queries, I'm now inclined to leave alone a feature that seems to be
> serving everyone quite well :)
>
> Thanks for the offer of help
>
> peace
>
> Gavin
>

Re: [Hibernate-devel] Queries on objects with composite keys...

[Gavin_King/Cirrus%<CI...@ci...>]

>> I am correct in my findings that you cannot currently do queries based
on composite keys where you only want to select using only 1 of the
composite key properties?  The only way currently supported is to do a
"find" for the object using the composite key object, right?

Yeah, thats about right. Nor can you frame queries that would require a
table join on the composite key.

>> I ended up patching Hibernate so you can do a select based on the
composite key, however I think the way I did it is only valid as a
temporary solution.  I would be willing to work on adding complete query
support for composite keys if anyone else thinks there is demand for this.

Sounds great! Any work you could do in this area is _much_
appreciated!Exactly what are you proposing? I'm thinking a syntax something
like:

from foo in class eg.Foo where foo.id.subid = 69

where id is the composite identifier of eg.Foo.

I apologise for all the hand-written parsing code for the query language. I
knew zero about lexing + parsing at the time so I ended up doing by hand
what I should have used ANTLR for. It was my intention to reimplement the
query language using an antlr grammar I wrote later. (Yeah, I educated
myself since.) However, since this is the very first feature request to do
with queries, I'm now inclined to leave alone a feature that seems to be
serving everyone quite well :)

Thanks for the offer of help

peace

Gavin

[Hibernate-devel] Queries on objects with composite keys...

[Jon L. <jon...@xe...>]

Hello,

I have been playing with Hibernate for the past few days and have been =
very pleased with the speed at which I can get things up and running.  I =
am using it to access some legacy data that has composite keys for some =
of the tables.

I am correct in my findings that you cannot currently do queries based =
on composite keys where you only want to select using only 1 of the =
composite key properties?  The only way currently supported is to do a =
"find" for the object using the composite key object, right?

If it is supported I couldn't figure out how to do it, so I ended up =
patching Hibernate so you can do a select based on the composite key, =
however I think the way I did it is only valid as a temporary solution.  =
I would be willing to work on adding complete query support for =
composite keys if anyone else thinks there is demand for this.

Regards,
Jon Lipsky

[Hibernate-devel] 0.9.11 fix redux

[Anton v. S. <an...@ap...>]

I noticed that the external connection done for 0.9.11 wasn't complete: the
else clause in RelationalDatabaseSession.disconnect() contained "return
conn", which still returned a null.  I've changed it to "return c", which
returns the connection.

This was a really easy change to miss in my original email - I should have
commented it...

Anton

[Hibernate-devel] minor change: Jikes & SortedSet

[Anton v. S. <an...@ap...>]

In checking in the change to SortedSet, I also took the liberty of adding
"this." qualifiers to the references to the variable "set" in the inner
class SubSetProxy.  This shouldn't have any impact on behavior.

The Jikes compiler was warning that these references were ambiguous.  The
compiler's being overzealous, I think, but I just wanted to eliminate the
warnings.  (SubSetProxy inherits "set" from its parent SetProxy, and the
outer class SortedSet also inherits a "set" variable from its parent, Set.)

Anton

RE: [Hibernate-devel] SortedSet & SortedMap bugfix (?)

[Anton v. S. <an...@ap...>]

I've just checked in the below change to SortedSet and SortedMap.


> -----Original Message-----
> From: Anton van Straaten [mailto:an...@ap...]
> Sent: Tuesday, May 14, 2002 3:19 AM
> To: Gavin_King/Cirrus%CI...@ci...
> Cc: hib...@li...
> Subject: RE: [Hibernate-devel] SortedSet & SortedMap bugfix (?)
> 
> 
> > (2) just comment out SortedMap.copy() and SortedSet.copy()
> > and use the implementations from the superclasses Map and Set.
> > Looking at the way the snapshots are used, we don't need the copy
> > to belong to the subclass.....
> 
> This makes better sense - less code is good!  
> 
> > Feel welcome to commit either of those fixes .... otherwise I will
> > do it this evening.
> 
> I won't get to checking this in until "tomorrow" (about ten hours 
> from now), but I'm sure it can wait till then.
> 
> Anton
> 

RE: [Hibernate-devel] SortedSet & SortedMap bugfix (?)

[Anton v. S. <an...@ap...>]

> (2) just comment out SortedMap.copy() and SortedSet.copy()
> and use the implementations from the superclasses Map and Set.
> Looking at the way the snapshots are used, we don't need the copy
> to belong to the subclass.....

This makes better sense - less code is good!

> Feel welcome to commit either of those fixes .... otherwise I will
> do it this evening.

I won't get to checking this in until "tomorrow" (about ten hours from now),
but I'm sure it can wait till then.

Anton

[Hibernate-devel] What I've Been Doing, etc.

[Gavin_King/Cirrus%<CI...@ci...>]

Just to let people know what I've been working on:

1. I've done a lot of work on the new "proxies" feature
- enabling arrays of proxies
- fixing a couple of little bugs
- integrating proxies into the existing test suite
2. Fixing a bug in the "deferred loading" feature

I'm not sure if anyone knows much about the deferred loading thing.
Basically, when executing a non-polymorphic query, hibernate holds
off on loading associated objects of the same class (ie. the class
returned by the query) just in case they are already in the query
result set. This was a very difficult feature to get working and
apparently I still didn't have it 100% right.

On a related subject, I think I should better document the
hibernate.use_outer_join feature which lets you load multiple
associated objects in one SELECT if your database supports
ANSI-style joins. I'm half tempted to enable this feature by
default for some databases by addding Dialect.hasOuterJoins().

Re: [Hibernate-devel] SortedSet & SortedMap bugfix (?)

[Gavin_King/Cirrus%<CI...@ci...>]

>
>    java.util.SortedSet clonedSet = new TreeSet();
>
> This looks to me as though it should read:
>
>    java.util.SortedSet clonedSet = new TreeSet(comparator);
>
> ...which fixes the problem for me.  There's an equivalent line
> in SortedMap which I expect suffers from the same problem.

Ah. Yeah, actually the "clonedSet" is used only as a snapshot and
we dont care about the sorted order of the snapshot - so we wouldn't
really need the comparator, except if the elements don't implement
Comparable at all.

I can think of two different fixes:

(1) Your suggestion.

(2) just comment out SortedMap.copy() and SortedSet.copy()
and use the implementations from the superclasses Map and Set.
Looking at the way the snapshots are used, we don't need the copy
to belong to the subclass.....

> I won't make this my first change in CVS, since I haven't attempted
> to understand the SortedSet logic in any detail, and for all I know
> this change could affect something else.

Feel welcome to commit either of those fixes .... otherwise I will
do it this evening.

Thanks (once again)

Gavin

[Hibernate-devel] SortedSet & SortedMap bugfix (?)

[Anton v. S. <an...@ap...>]

I was crashing trying to load a SortedSet, with "java.lang.RuntimeException:
Hibernate failed trying to load a collection".  The log showed that a
ClassCastException was occurring in java.util.TreeMap.compare.  It turned
out that this was because the TreeMap in question didn't have a comparator,
although I had specified one.  I traced this to line 24 of SortedSet.java
(in CVS & v0.9.11):

	java.util.SortedSet clonedSet = new TreeSet();

This looks to me as though it should read:

	java.util.SortedSet clonedSet = new TreeSet(comparator);

...which fixes the problem for me.  There's an equivalent line in SortedMap
which I expect suffers from the same problem.

This problem presumably only occurs if the objects being compared don't
support the Comparable interface - if they do, TreeMap.compare uses that.

I won't make this my first change in CVS, since I haven't attempted to
understand the SortedSet logic in any detail, and for all I know this change
could affect something else.

Anton

RE: [Hibernate-devel] not so fast! :)

[Gavin_King/Cirrus%<CI...@ci...>]

> Hey, these would only be embarrassing if they were caught after
> v1.0!

Yeah, but I'm kinda hoping 1.0 is fairly close now.....

> I figured the external connection scenario wasn't being widely
> used.  I probably wouldn't be using it either, except that I'm
> integrating into an existing system, and I thought it would be
> a good idea to share the same connection pool as the rest of
> the system.

Exactly the kind of usage I had in mind :)

> > I guess this means I have to do another release real soon ... but
> > not today.

> I agree it's not that urgent, unless you're aware of anyone
> embedding the code into heart-lung machines as we speak... :)

heh :) heart-lung machine programmers can use the version in CVS

> I was thinking about that: testing the finalize issue would be
> tricky, since the symptoms could range from benign to unpredictable.
> I think this particular issue would be handled better by some
> additional checks (or assertions).  One would be inside
> RelationalDatabaseSession.finalize:

Okay, I integrated that...

> Another place to check for and warn about a non-null connection
> would be inside cleanup(), i.e.

probably not necessary - that stuffs better done in the tests
(as you noted).

Unfortunately the various possible failure modes make this section
of the code (ie. Session.close() + the deprecated versions) very
fiddly and difficult to fully test.

> My first bug, with the parameters swapped around, is easier to unit
> test for.  I'll see if I can put something together after I've
> installed the code I'm currently working on.  (Unit test after
> release, XLP - Xtremely Lazy Programming?)

:)

Anton, if you send me your sourceforge username, I will give you CVS
access.

RE: [Hibernate-devel] not so fast! :)

[Anton v. S. <an...@ap...>]

Hey, these would only be embarrassing if they were caught after v1.0!  To
Hibernate's credit, this latest one at least gave a log warning.  I figured
the external connection scenario wasn't being widely used.  I probably
wouldn't be using it either, except that I'm integrating into an existing
system, and I thought it would be a good idea to share the same connection
pool as the rest of the system.

> I guess this means I have to do another release real soon ... but
> not today.

I agree it's not that urgent, unless you're aware of anyone embedding the
code into heart-lung machines as we speak... :)

> As a high priority, I need to write some
> tests for this functionality.

I was thinking about that: testing the finalize issue would be tricky, since
the symptoms could range from benign to unpredictable.  I think this
particular issue would be handled better by some additional checks (or
assertions).  One would be inside RelationalDatabaseSession.finalize:

	if (conn!=null) {
		log.warn("Unclosed session");
		if (conn.isClosed())
			log.warn("Inconsistent state: unclosed session with closed connection");
		else
			conn.close();
	}

The second warning triggers in my case - that's how I verified what was
going on.

Another place to check for and warn about a non-null connection would be
inside cleanup(), i.e.

	if (conn!=null) log.warn("Unclosed connection");

I believe this would also have triggered in my case, although I didn't try
it.  I guess you could also exercise this kind of thing with some white-box
unit tests.

My first bug, with the parameters swapped around, is easier to unit test
for.  I'll see if I can put something together after I've installed the code
I'm currently working on.  (Unit test after release, XLP - Xtremely Lazy
Programming?)

Anton

Re: [Hibernate-devel] not so fast! :)

[Gavin_King/Cirrus%<CI...@ci...>]

> Thanks for the quick response on 0.9.11!  Except it makes me feel
> a bit ungrateful to have to report another bug now - hopefully
> this'll be my last one for a while.

I sure hope its the last, if only to save _me_ further embarrasment!
I've applied your suggested fix to CVS, and also fixed the deprecated
versions. At some stage along the way I stopped testing hibernate in
the supply-your-own connection scenario - a bad decision which has
opened us up to these bugs. As a high priority, I need to write some
tests for this functionality.

> BTW, this problem might seem fairly benign - just a log warning,
> as long as your connections don't mind being closed twice.  But
> in theory, it would be possible for finalize() to trigger after
> an externally-pooled connection has been reactivated, and thus
> close an active connection.  That might have been
> fun to track down!  ;)

Urgghhh. Thanks for picking it up at this point.

I guess this means I have to do another release real soon ... but
not today. I might do a next release after I integrate Christoph
Beck's datatype path - which was my next task.

Many Thanks

Gavin.

[Hibernate-devel] not so fast! :)

[Anton v. S. <an...@ap...>]

Thanks for the quick response on 0.9.11!  Except it makes me feel a bit
ungrateful to have to report another bug now - hopefully this'll be my last
one for a while.  This is a related problem, which affects 0.9.9 and
earlier.  I haven't yet actually tested it with 0.9.11, but the relevant
code looks the same.

Once the autoclose problem was fixed, I started getting warnings in my log
(the new logging is working beautifully, btw.)  The warnings were from
RelationalDatabaseSession.finalize(), about an "unclosed connection".
Investigation showed that the connection in question was in fact already
closed (by me, externally), and that finalize() was closing it a second
time.  Resin's connection wrapper didn't seem to mind this, so it didn't
actually cause any direct symptoms for me, other than the log warning.

The real problem seems to be that disconnect() doesn't set conn=null when
autoclose is off, which leads finalize() to think that the session still has
a valid connection.  I fixed it by changing the try block in disconnect() to
the following:

	try {
		closeStatements();
		Connection c = conn;
		conn=null;
		if (autoClose) {
			factory.closeConnection(c);
			return null;
		}
		else {
			return c;
		}
	}

This seems to fix it, at least in my tests.  I don't think it's necessary to
add any guards to finalize() itself, as long as conn gets set to null when
it's supposed to.

A similar problem exists in commit() and cancel(), but I didn't bother to
fix them since they're deprecated.  Of course, they're trivial to fix.

BTW, this problem might seem fairly benign - just a log warning, as long as
your connections don't mind being closed twice.  But in theory, it would be
possible for finalize() to trigger after an externally-pooled connection has
been reactivated, and thus close an active connection.  That might have been
fun to track down!  ;)

Anton

[Hibernate-devel] version 0.9.11 is up

[Gavin_King/Cirrus%<CI...@ci...>]

Go get it! This version includes Anton's connection handling bugfix.

Re: [Hibernate-devel] sneaky argument order bug in RelationalDatabaseSessionFactory

[Gavin_King/Cirrus%<CI...@ci...>]

Oh yuck! Wow, I'm _amazed_ this wasn't discovered earlier.
What an awful bug! I guess people just havn't been using
various combinations of functionality, so havn't noticed.

I guess it went unnoticed because autoclose is only
used by deprecated methods, and autocommit defaults to
true - which would be the correct value of autoclose
under the most common usage of hibernate.

Oh well.

I will release a new version just for that bugfix.

Thanks Anton.

> In the method RelationalDatabaseSessionFactory.openSession(
> Connection conn, boolean autoClose, long timestamp), the
> autoClose and autoCommit arguments being passed to the
> RelationalDatabaseSession constructor are the wrong way
> around, and need to be swapped.

> The symptom, in my case, was that I was supplying an external
> connection to openSession, but Hibernate was returning null from
> Session.close(), instead of the external connection.  In addition,
> it was apparently retaining the connection for its pool (in
> DriverManagerConnectionProvider.closeConnection).

[Hibernate-devel] sneaky argument order bug in RelationalDatabaseSessionFactory

[Anton v. S. <an...@ap...>]

This bug applies to the code currently in CVS, and goes back to at least
v0.9.8:

In the method RelationalDatabaseSessionFactory.openSession(Connection conn,
boolean autoClose, long timestamp), the autoClose and autoCommit arguments
being passed to the RelationalDatabaseSession constructor are the wrong way
around, and need to be swapped.

The symptom, in my case, was that I was supplying an external connection to
openSession, but Hibernate was returning null from Session.close(), instead
of the external connection.  In addition, it was apparently retaining the
connection for its pool (in
DriverManagerConnectionProvider.closeConnection).

This meant that connections from my servlet engine's pool (Resin) were being
drained into Hibernate's pool, eventually causing Resin to go into a serious
sulk when it found that its swimming privileges had been revoked.  :)

I imagine this may be causing other symptoms related to autoCommit.  Calling
openSession with no arguments would set autoCommit to true, and calling it
with a connection would set it to false.

Anton

Re: [Hibernate-devel] Problem: MySQL large string/binary columns

[Christoph B. <be...@od...>]

> 
> I had a look at the way you implemented this. Its really nice,
> actually. I'm still not 100% convinced that it needs to be
> automatic but I cant think of any compelling reason why it should
> _not_ be so I'm inclined to use your patch.
> 
Thanks.
> The main difficulty with this is updating all 10 existing
> dialects. hmmmmmm.....
> 
Yes. But there's no need to do it now. The patch doesn't
break any existing code. In fact, I don't even know if it's
nessecary for other dbms. But if so, there's now an easy
way to do it.

Re: [Hibernate-devel] Problem: MySQL large string/binary columns

[Gavin_King/Cirrus%<CI...@ci...>]

I had a look at the way you implemented this. Its really nice,
actually. I'm still not 100% convinced that it needs to be
automatic but I cant think of any compelling reason why it should
_not_ be so I'm inclined to use your patch.

The main difficulty with this is updating all 10 existing
dialects. hmmmmmm.....