Ok, if backwards compatibility is only broken for people using postgres, I'm fine changing the smallint-->boolean. Modules and core could continue to work with voided=1/voided=true in mysql. You should find/change as many places in core and change to just voided=true so that at least core works with postgres. If a module wants to support postgres they'll have to change their own sql to voided=true.
Ben On Tue, Aug 30, 2011 at 12:14 PM, Saptarshi Purkayastha <[email protected]>wrote: > Ben, > > Then we will have to change the other part where instead of true or false, > or setBoolean/getBoolean, we will have to change those to setInt/getInt, > This will be needed to be done in HSQL, ResultSet and other places because > there is inconsistency on either sides. So the changes have to be done one > way or the other, if you have to make it work in different database engines. > > Since our Objects have Boolean properties, I suggested that we make the > columns liquibase boolean from smallint. It helps keep someone watching the > columns sane... > > > > --- > Regards, > Saptarshi PURKAYASTHA > > My Tech Blog: http://sunnytalkstech.blogspot.com > You Live by CHOICE, Not by CHANCE > > > On 30 August 2011 12:33, Ben Wolfe <[email protected]> wrote: > >> Roger, this is all hidden behind the scenes in the dao layer. The api >> methods and objects do not need to change regardless of what decision is >> made. The conversation you are thinking of was also started by Saptarshi: >> "Why do we have '(big b) Boolean isVoided()' *AND* '(big b) Boolean >> getVoided()'?" >> >> The best approach really is to just leave it as we have it: store all >> booleans as integers and refer to them as integers in the code. This means >> we don't break backwards compatibility (which modules developers HATE) and >> also that we're compatible with all database engines. >> >> Burke/Saptarshi: That regex won't find queries created with Criteria >> queries or with HSQL. >> >> Ben >> >> >> On Mon, Aug 29, 2011 at 9:20 PM, Friedman, Roger (CDC/CGH/DGHA) (CTR) < >> [email protected]> wrote: >> >>> Wouldn't this also change all of our get methods to is methods? I know >>> we had a thread about this recently but can't seem to find it.**** >>> >>> ** ** >>> >>> *From:* [email protected] [mailto:[email protected]] *On Behalf Of *Saptarshi >>> Purkayastha >>> *Sent:* Monday, August 29, 2011 1:13 PM >>> *To:* [email protected] >>> *Subject:* Re: [OPENMRS-DEV] Changes to liquibase xmls**** >>> >>> ** ** >>> >>> Interesting suggestion Burke, to use NOT... and it works for MySQL, >>> Postgres and H2... but alas not in SQL Server and Oracle (probably DB2 as >>> well)!**** >>> >>> The voided='0' seems to be work on all databases**** >>> >>> ** ** >>> >>> Also there are many more boolean columns than retired and voided. We also >>> use voided = false at some places. Nonetheless that search-and-replace could >>> be the first change.**** >>> >>> ** ** >>> >>> PS: This <http://troels.arvin.dk/db/rdbms/> has been a seminal document >>> that I've used lately.**** >>> >>> >>> --- >>> Regards, >>> Saptarshi PURKAYASTHA >>> >>> My Tech Blog: http://sunnytalkstech.blogspot.com >>> You Live by CHOICE, Not by CHANCE >>> >>> **** >>> >>> On 29 August 2011 22:00, Burke Mamlin <[email protected]> wrote:** >>> ** >>> >>> Is there any reason *not* to be using "where voided" and "where not >>> voided" in queries instead of comparing to a number? Hopefully >>> s/(voided|retired)\s*=\s*1/$1/g and s/(voided|retired)\s*=\s*0/not $1/g >>> would be reasonably efficient and specific, making finding and fixing most >>> of the code relatively easy. Checkstyle could look for this pattern & throw >>> warnings stating the preferred convention for checking boolean values. I'm >>> sure there are other boolean columns to be addressed and a simple search & >>> replace might not fix them all, but there's no reason that they need to be >>> all fixed in one step – i.e., (1) fix the low-hanging fruit, (2) put >>> conventions ± checkstyle checks to reduce the likelihood that the problem >>> will continue to grow, and then (3) seek out and fix the remaining cases >>> focusing on core & distributed modules first.**** >>> >>> ** ** >>> >>> -Burke**** >>> >>> ** ** >>> >>> On Mon, Aug 29, 2011 at 9:26 AM, Michael Seaton <[email protected]> wrote: >>> **** >>> >>> I share Saptarshi's concern, particularly if there are lots of modules >>> out there that may be making the "voided = 0" assumptions in direct SQL >>> queries. Will this be something that is not backwards compatible and >>> requires lots of module changes in order to successfully upgrade?**** >>> >>> >>> >>> >>> On 08/29/2011 02:18 AM, Ben Wolfe wrote: **** >>> >>> The changes should be down at the db layer, right? I think that should >>> be fine. >>> >>> Is there a way to tell postgres to treat 0 like a 'false'? Perhaps with >>> a db connection string property? >>> >>> Ben**** >>> >>> On Sun, Aug 28, 2011 at 5:42 AM, Saptarshi Purkayastha <[email protected]> >>> wrote:**** >>> >>> One of the big problems that I'm encountering at the moment for making >>> OpenMRS run on most database servers is with our assumption of the BOOLEAN >>> type. Because MySQL stores BOOLEAN as TINYINT(1), we have made the >>> assumption that BOOLEAN will be represented as 0 or 1. Thus, the column type >>> for (retired, voided etc) in liquibase was using smallint (to make it >>> ANSI-SQL :D) as the type and default as 0 (FALSE). This was fairly easy to >>> change in the liquibase xmls by a search-and-replace (but still looking up >>> all the columns) **** >>> >>> ** ** >>> >>> The bigger issue surrounding this assumption is in code, where we have >>> made voided=0 in queries or getInt() from ResultSet. These are fine for the >>> MySQL assumption, but doesn't work with other databases. This is a fairly >>> big change to do, with changes at many places in code...**** >>> >>> ** ** >>> >>> Are we ready?? **** >>> >>> **** >>> >>> --- >>> Regards, >>> Saptarshi PURKAYASTHA >>> >>> My Tech Blog: http://sunnytalkstech.blogspot.com >>> You Live by CHOICE, Not by CHANCE >>> >>> **** >>> >>> On 18 August 2011 15:59, Ben Wolfe <[email protected]> wrote:**** >>> >>> The funny thing is that the liquibase changeset files were used for the >>> initial scripts was for database portability. If you can export them as >>> standard sql files and run those just as easily as the liquibase ones, go >>> for it. The tricky thing to solve would be how to get the progress bars to >>> work in the initialization wizard. >>> >>> You can either change the datatypes to a generic ansi standard ones, or >>> change to something that liquibase knows about and converts to each >>> different datatype on the different dbms's correctly. >>> >>> Ben **** >>> >>> ** ** >>> >>> On Wed, Aug 17, 2011 at 10:47 PM, Saptarshi Purkayastha < >>> [email protected]> wrote:**** >>> >>> MySQL longtext is the same as the TEXT of postgresql... for the others we >>> have to make database specific options between the data types **** >>> >>> tinyint to smallint in the liquibase-schema-only**** >>> >>> there are many similar difference that are database specific and we have >>> to make database specific datatypes.**** >>> >>> ** ** >>> >>> I'm yet to go through all the changes required... but will make as these >>> come. But for datatypes that are incompatible, probably be database >>> specific.**** >>> >>> Is that ok?? or we should move those to ANSI-standard ones only??**** >>> >>> ** ** >>> >>> >>> --- >>> Regards, >>> Saptarshi PURKAYASTHA >>> >>> My Tech Blog: http://sunnytalkstech.blogspot.com >>> You Live by CHOICE, Not by CHANCE >>> >>> **** >>> >>> On 18 August 2011 00:48, Darius Jazayeri <[email protected]> >>> wrote:**** >>> >>> Hi Saptarshi, **** >>> >>> ** ** >>> >>> In principle I'm fine with these, but let's get at the specifics of >>> datatypes.**** >>> >>> ** ** >>> >>> We probably use the mysql text, mediumtext, and longtext types a lot, >>> because they're really usefully-sized datatypes. What will we replace those >>> with?**** >>> >>> ** ** >>> >>> What other datatypes are you expecting to change?**** >>> >>> ** ** >>> >>> -Darius**** >>> >>> ** ** >>> >>> On Wed, Aug 17, 2011 at 11:25 AM, Saptarshi Purkayastha < >>> [email protected]> wrote:**** >>> >>> Hi, **** >>> >>> ** ** >>> >>> While we were recently discussing that OpenMRS should run on multiple >>> database servers, I started working on Support Multiple Databases in >>> OpenMRS Installation and >>> Update<https://tickets.openmrs.org/browse/TRUNK-1925> >>> .**** >>> >>> There are some of the following broad changes that need to be made the >>> the liquibase xml so that we can install OpenMRS on different databases >>> (target MySQL, Postgres, MsSQL... and may be later Oracle).**** >>> >>> ** ** >>> >>> 1.) Change datatypes and create tables that are compatible with all the >>> databases (directly for compatible types or database specific for >>> non-compatible types)**** >>> >>> 2.) Remove precision from many columns which are not compatible across >>> all those database servers**** >>> >>> 3.) Divide a larger changeset into smaller changeset so that they can be >>> done commonly (by changing syntax) across multiple database servers.**** >>> >>> 4.) Due to move to liquibase 2.0, all checksums for changesets have been >>> NULL'd and then changed to the new format. This should happen automatically, >>> but if anyone depends on these checksums, then you should reply to this >>> email :)**** >>> >>> ** ** >>> >>> We may also have to change if anywhere non-ANSI syntax or MySQL-syntax >>> has been used in the DAO. I haven't reached that far to tell how many such >>> instances exist, but I hope there aren't many such places**** >>> >>> ** ** >>> >>> What do other developers think about these changes? Any suggestions on >>> the way?? Anything that u think should be avoided or done?? These are fairly >>> large changes and may result in incompatible checksums for already run >>> changesets.**** >>> >>> ** ** >>> >>> ** ** >>> >>> PS: On a sidenote, I would like to highlight that for new installations >>> it is silly that we are doing changesets based installations. While the >>> world has moved to image based deployments for OS and large-software >>> packages, we did the reverse and moved to changesets based installations. >>> Upgrades are easier through changesets, but for new installations they are >>> lengthy and boring. I would like to propose the for new installations, just >>> an sqldump deploy is easy and fast, while keeping to changesets for >>> upgrades.**** >>> >>> >>> --- >>> Regards, >>> Saptarshi PURKAYASTHA >>> >>> My Tech Blog: http://sunnytalkstech.blogspot.com >>> You Live by CHOICE, Not by CHANCE**** >>> ------------------------------ >>> >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> **** >>> >>> ** ** >>> >>> ** ** >>> ------------------------------ >>> >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> **** >>> >>> ** ** >>> >>> ** ** >>> ------------------------------ >>> >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> **** >>> >>> ** ** >>> ------------------------------ >>> >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> **** >>> ------------------------------ >>> >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> **** >>> >>> ** ** >>> >>> ** ** >>> ------------------------------ >>> >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> **** >>> >> >> > ------------------------------ > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
