Don't be afraid to use several pull requests (smaller -> easier to review/understand -> faster to apply).
You seem to know your way around firebird - if option 2 works and you recommend it, it's good enough for me at least. /Oskar 2014/1/10 Amro El-Fakharany <[email protected]> > 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.
