I've merged PR #249. But on the build server the number of failing tests was already down to 80, and didn't change.
/Oskar 2014-01-21 11:40 GMT+01:00 Amro El-Fakharany <[email protected]>: > Sorry everyone but I was plain wrong! > The Firebird driver doesn't have any pooling issues related to our > discussion here and there is no need to change any configurations! > > The trouble is in the implementation of IncrementGenerator wich creates a > brand new connection every time it needs to fetch the next value. > Examples of failing tests are EntityWithInverseManyToManyTest and > EntityWithInverseOneToManyJoinTest. Both of those Fixtures needs to be run > in one shot to see one of them fail! > > I have opened a jira ticket https://nhibernate.jira.com/browse/NH-3591and > created a pull request > https://github.com/nhibernate/nhibernate-core/pull/249 > > Now on my machine this lets the failing tests drops from 455 (after > merging the last changes) down to 99. > > Amro > > > On Friday, January 10, 2014 12:03:28 PM UTC+1, Amro El-Fakharany wrote: >> >> Done as discussed so far: https://github.com/nhibernate/ >> nhibernate-core/pull/246 >> >> I also need to make a small change to the firebird configuration template >> for the following reason (it is not included in the pull request yet): >> When there are more than one connection to a database firebird will >> regect any Drop or Alter table statements from one connection if the table >> was/is used by any of other connections. >> Now The Firebird driver has some issues with connection pooling in such >> it sometimes opens a brand new connection instead of returning one from the >> pool. >> This results in a lot of tests failing to drop the schema after they're >> done. This in turn causes the following tests to fail because the fixture >> setup raises an exception complaining about tables being already created. >> More than 200 tests are failing because of this! >> >> We have basically two* options: >> 1) Turn pooling off. This will make the tests run very, very slow but >> should be relatively safe. >> 2) Force the driver to run under embedded mode. I don't know precisely >> why, but the pooling issues disappear in this mode and the tests are still >> running fast. The drawback ist that the driver may change this in furture >> releases. >> Both of these options can be set in the driver configuration template and >> we're done. >> >> - Which option would you guys choose? Personally I would go for the >> second one. >> - Can I include it in the same pull request above? >> >> *There is actually a third option I'm thinking about which is to >> contribute to the driver project and fix that annoying pooling issue but >> I'm not sure if I can do that and even if I can it won't happen soon. >> >> Amro >> >> On Monday, December 23, 2013 11:23:27 AM UTC+1, Amro El-Fakharany wrote: >>> >>> Hi Everybody, >>> I noticed that there are some (actually a lot!) tests broken for >>> Firebird and I decided to tackle each Problem one by one. >>> >>> So far I have found three Issues wich are (roughly description): >>> 1) Decimal data type: Firebird does'nt support precisions larger than 18 >>> and the default of nh is 19. >>> I extended the FirebirdDialect to override the "GetTypeName" method and >>> swap larger Decimal precisions with 18. >>> >>> 2) Parameters Within a select clause: Firebird needs a typecast if a >>> parameter is used within a select clause. >>> I extended the FirebirdClientDriver to override the AdjustCommand method >>> and typecast the parameter. >>> >>> 3) Temporary Tables: Firebird supports temporary Tables, which must be >>> created in an isolated transaction, and so I thought it is just a matter of >>> extending the dialect and overriding the related properties and methods. >>> Running the tests afterwards I discovered that this was'nt the case. >>> >>> After digging deeper in the code I think I now know what's going on and >>> I think the issue is a "nasty" one and not necessarily related to Firebird. >>> This is going to be a rather long Description but please bear with me >>> since I really need some guidance here! >>> >>> I'll start with one failing test which is found in >>> *AbstractEntityWithOneToManyTests.OneToManyCollectionOptimisticLockingWithUpdate()* >>> . >>> The relevant Part is at the end of the test: >>> s = OpenSession(); >>> *t = s.BeginTransaction();* <== keep this in mind. >>> >>> c = s.CreateCriteria<Contract>().UniqueResult<Contract>(); >>> *s.CreateQuery("delete from Party").ExecuteUpdate();* >>> >>> An Exception is thrown at s.CreateQuery().ExecuteUpdate(). >>> >>> Debuging and "Following the dots" you reach >>> *MultiTableDeleteExecutor.Execute(parameters, >>> session)* with: >>> CoordinateSharedCacheCleanup(session); >>> *CreateTemporaryTableIfNecessary(persister, session);* >>> >>> the implementation of CreateTemporarayTableIfNecessary() in >>> AbstractStatementExecutor is: >>> IIsolatedWork *work = new >>> TmpIdTableCreationIsolatedWork(persister, log, session);* >>> if (ShouldIsolateTemporaryTableDDL()) >>> { >>> *if >>> (Factory.Settings.IsDataDefinitionInTransactionSupported)* >>> { >>> *Isolater.DoIsolatedWork(work, session);* >>> } >>> else >>> { >>> Isolater.DoNonTransactedWork(work, session); >>> } >>> } >>> else >>> { >>> work.DoWork(session.ConnectionManager.GetConnection(), >>> null); >>> session.ConnectionManager.AfterStatement(); >>> } >>> >>> The bold if Statement is absolutly useless! The Property >>> IsDataDefinitionInTransactionSupported is never used (let alone set to >>> true!) anywhere in the entire code base and hence it is always false. >>> I commented out the whole thing except *Isolator.DoIsolatedWork(work, >>> session)* - because this is what firebird requires - just to see what >>> will happen which was unfortunatly nothing. The test still fails with the >>> same exception. >>> >>> Digging further starting from *Isolator.DoIsolatedWork(work, session) *you >>> reach AdoNetTransactionFactory.ExecuteWorkInIsolation(ISessionImplementor >>> session, IIsolatedWork work, bool transacted) with the relevant >>> implementaion part of (i removed the comments for brevity): >>> if (session.Factory.Dialect is SQLiteDialect) >>> connection = session.Connection; >>> else >>> connection = session.Factory.ConnectionProvider. >>> GetConnection(); >>> >>> if (transacted) >>> { >>> trans = connection.BeginTransaction(); >>> } >>> >>> work.DoWork(connection, trans); >>> >>> if (transacted) >>> { >>> trans.Commit(); >>> } >>> Notice how It's asking for a new connection and starting a new >>> transaction (wich is precisly what it should do) if requested. It then >>> passes both to work.DoWork. In our case here the "work" is >>> TmpIdTableCreationIsolatedWork.DoWork(*IDbConnection connection*, >>> *IDbTransaction >>> transaction*) with the relevant implementation: >>> stmnt = *session.ConnectionManager.CreateCommand()*; >>> stmnt.CommandText = persister.TemporaryIdTableDDL; >>> stmnt.ExecuteNonQuery(); >>> session.Factory.Settings. >>> SqlStatementLogger.LogCommand(stmnt, FormatStyle.Ddl); >>> >>> The code is not only *ignoring* the passed in connection and >>> transaction but is asking for a command from the ConnectionManager which >>> will return a command assigned to a *different* connection from that >>> passed in! >>> >>> Further more the ConnectionManager.CreateCommand() will assign the >>> command the very first transaction created by the test and the whole thing >>> blows up! >>> Here ist the implementation of ConnectionManager.CreateCommand(): >>> var result = *GetConnection().CreateCommand();* >>> *Transaction.Enlist(result);* >>> return result; >>> >>> Now I think the solution is actually simple, namly refactor the >>> TmpIdTableCreationIsolatedWork.DoWork to use the passed in parameters, >>> but I need to make sure that other dialects will not be affected. >>> (After all there must be a reason why the original author(s) ignored the >>> passed in parameters right?) >>> >>> 1) Does any dialect support DDL statements within the same transaction >>> as the one which actually usese them (such as selects or inserts) at all? >>> 2) If yes, will they be broken if we did the refactoring or do they also >>> support DDL in separate transactions? >>> 3) Can we remove the *if >>> (Factory.Settings.IsDataDefinitionInTransactionSupported) *and instead >>> just stick with *Isolater.DoIsolatedWork(work, session);*? >>> >>> Sorry if the post was rather long and Merry Christmas to all of you. >>> >> -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. > -- --- You received this message because you are subscribed to the Google Groups "nhibernate-development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
