The changes or "stuff" I was talking about don't include code changes but tweaking of some mapping or configuration files. The code itself is untouched.
Regarding the example I gave with the DateTimeOffset: Every test in a fixture that is using a mapping with that type will break regardless of whether individual tests are targeting that field or not. The reason is that the database won't be able to create the schema at *fixture* setup because of that unknown data type. The result is that actually valid tests targeting something completely unrelated to that field will also fail! Amro Am 09.01.2014 17:40 schrieb "Oskar Berggren" <[email protected]>: > It can be acceptable to collect various small but (somehow) related > changes under a single ticket and pull request - don't be afraid to use > multiple commits if it makes sense though. > > > As for the test with DateTimeOffset - if the use of this type is beside > the point of the test it could be rewritten to use a more generally > supported type. If on the other hand it's related to what is actually being > tested, the test should be marked as ignored for the firebird dialect > (using Assert.Ignore() or similar). > > /Oskar > > > > > 2014/1/9 Amro El-Fakharany <[email protected]> > >> Oskar, >> >> I will isolate the change fort the first issue and submit a pull request >> for it. >> >> As I mentioned in a former post the second issue is still unsolved (or >> actually only partially solved) so I will leave that one open. >> >> I will also create a third ticket in Jira regarding the temporary table >> issue, isolate the changes for that and submit a pull request for it. >> >> >> >> But… >> >> There are also some “small” stuff that makes tests break for firebird and >> I don’t think it would be wise to open a Jira ticket for each one of them >> and then submit a separated pull request for the related change. >> >> One such an example is using (the utterly weird) data type >> “DateTimeOffset”** in a mapping for one test (I can’t remember exactly >> which one right now) which lets that test fail under Firebird (and any DB >> that doesn’t support it). When I delete or comment out the related field in >> the mapping file all the test for **MS-SQL** -which is one of the rare >> engines if not even the only one that supports that thing- are still >> passing! >> >> Opening a ticket for such a case doesn’t make a lot of sense to me. >> >> How do you or Alexander see it? >> >> >> >> ** Why is it supported as a “core” data type at all? Anyone can map it >> easily with a user type! >> >> >> >> Amro >> >> >> >> *Von:* [email protected] [mailto: >> [email protected]] *Im Auftrag von *Oskar Berggren >> *Gesendet:* Donnerstag, 9. Januar 2014 14:38 >> >> *An:* [email protected] >> *Betreff:* Re: [nhibernate-development] Firebirds broken tests >> >> >> >> It would be good if you could isolate the changes for the first two >> issues into one commit each, and submit them (and only them) as a pull >> request on github (referencing the jira issue numbers). >> >> /Oskar >> >> >> >> 2014/1/9 Amro El-Fakharany <[email protected]> >> >> Hi Alexander, >> >> I'm afraid I didn't understand you quite right. I did post 2 links to my >> fork of the repository and the changes i've made! Did I misunderstood you? >> Of course I can make a pull request if it helps more, it's just that I >> consider the work done so far to be "in Progress". >> >> Amro >> >> >> >> *Von:* [email protected] [mailto: >> [email protected]] *Im Auftrag von *Alexander I. >> Zaytsev >> *Gesendet:* Donnerstag, 9. Januar 2014 07:38 >> *An:* [email protected] >> *Betreff:* Re: [nhibernate-development] Firebirds broken tests >> >> >> >> Ho Amro, >> >> >> >> Can you please share some code or make a pull request? >> >> >> >> Best Regards, >> >> Alexander >> >> >> >> 2014/1/9 Amro El-Fakharany <[email protected]> >> >> Hi Oskar, >> thanks for taking the time and looking at this. >> I created the first 2 Issues in Jira ( >> https://nhibernate.jira.com/browse/NH-3586 and >> https://nhibernate.jira.com/browse/NH-3587). >> The second issue is unfortunatly still not solved so I'll wait with the >> pull request. >> >> You can find my current work here: >> https://github.com/amroel/nhibernate-core/tree/fix_firebird >> with the list of current changes: >> https://github.com/amroel/nhibernate-core/compare/fix_firebird >> >> The changes includes the refactoring for the third Issue which is now >> working for both Firebird and MS-SQL. I have'nt tested it for other >> dialects but it should be working. >> The failing tests dropped from 500+x down to 54, but there is still a lot >> of work to do. >> >> >> On Wednesday, January 8, 2014 7:21:31 AM UTC+1, Oskar Berggren wrote: >> >> >> >> >> >> 2013/12/23 Amro El-Fakharany <[email protected]> >> >> >> >> 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. >> >> >> >> Excellent! >> >> >> >> >> 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. >> >> >> >> Please open separate issues in Jira and submit pull requests for this. >> >> >> >> >> 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. >> >> >> >> I read somewhere that Firebird supports transactional DDL - does it not >> include temporary tables in this? >> >> >> >> As for the below, the code in *TmpIdTableCreationIsolatedWork* was added >> in 2009 and appears to only be used in fairly limited scenarios, so it's >> not inconceivable that there are still bugs in it. It certainly looks a bit >> strange. You might have a look at the corresponding java code in Hibernate >> to see how that looks. >> >> [...] >> >> 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? >> >> >> >> Yes, Postgresql has a very robust implementation of mixing DDL and DML >> inside the same transaction. SQL Server too can handle many things, as can >> several other RDBMS. >> >> >> >> 2) If yes, will they be broken if we did the refactoring or do they also >> support DDL in separate transactions? >> >> >> >> Whatever refactoring is made, for the databases where the current >> behavior is correct, that behavior should remain. (even if the current >> connection happens to be accessed using a different parameter.) >> >> >> >> 3) Can we remove the *if >> (Factory.Settings.IsDataDefinitionInTransactionSupported) *and instead >> just stick with *Isolater.DoIsolatedWork(work, session);*? >> >> >> >> I haven't analyzed this. >> >> >> >> /Oskar >> >> -- >> >> --- >> 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. >> >> -- >> >> --- >> 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. >> >> -- >> >> --- >> 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. > -- --- 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.
