Mysql/win/linux and case sensitive table name

tmccormi wrote on Thursday, September 13, 2012:

We just found this issue when porting a existing database from a windows XAMPP install to a Ubuntu install.  Windows XAMMP, by default is NOT case sensitive and created the form_camos table with the name form_CAMOS.  Subsequently all the internal records referencing that form use the upper case.   When porting the sql dump to linux the table names are all lower case.  So… now you openemr thinks the table is missing.

ERROR: query failed: SELECT max(id) AS max FROM form_CAMOS WHERE pid = '591'
Error: Table 'emr_betterlife.form_CAMOS' doesn't exist

Googlng gets this recommendation …

Database and table names are not case sensitive in Windows, and case sensitive in most varieties of Unix or Linux.
to resolve the issue set the lower_case_table_names to 1

Now… fixing this is a pain as the construction of the table name inside openemr comes from storing the name in the form and creating the form_tablename on the fly… (no comment) … the data records, therefore have uppercase table names (at least CAMOS, maybe others too).

Might want to patch the xampp package with this lower_case_table_names=1 config to prevent this in the future.

-Tony

bradymiller wrote on Friday, September 14, 2012:

Hi,

Never good to see this kind of stuff :slight_smile:
To fix the database, though, didn’t you just have to caps the camos to CAMOS in the database tables? (ie. isn’t the data within forms and registers tables using the caps CAMOS??)

The problem is that doing that do xampp doesn’t really fix the problem, since other windows installs will still have this problem. The ideal fix would be to convert all tables(only a couple conttrib forms and CAMOS seem to be the culprit) to only lowercase and ensure all new tables are lowercase. Of course, this would also mean some codebase changes and some database changes.

Another thing I wonder is if we can place something in the sql query itself for when finding form dirname and on the registers call to ignore caps?? That would be the simplest solution.

-brady
OpenEMR

bradymiller wrote on Friday, September 14, 2012:

Hi,
Another option would be to run any queries that involve these tables through a special mysql query in sql.inc that will convert the incorrect table names (since the list of capped tables is very small and known) into the correct capped ones. Hopefully yehster doesn’t faint when hearing this idea considering all of the innefficiencies it may bring on; could even piggyback onto the audit engine :slight_smile:
-brady

tmccormi wrote on Friday, September 14, 2012:

What we did was dump the tables to text and run a sed find-replace to lowercase the CAMOS to camos  the reimport.   In any case the xampp default should be fixed for new installs.

Once the table has been created in specific case then  both sides have to match.  Constructing the table name instead of looking it up from a the registry based on a form id is the core flaw, but that’s very embedded now.

Tony

bradymiller wrote on Friday, September 14, 2012:

Hi Tony,

Figured out a way to fix this in the codebase, which required two things:

1. Bug fix to correctly collect sql errors:
http://github.com/bradymiller/openemr/commit/e5497774ef5617ab3c79cdf4f3d4afead4768963

2. Followed by a fix to allow flexibility in casing of certain tables:
http://github.com/bradymiller/openemr/commit/0269c2bfb5d50ffdb37bd4bac312bab5a427e45e

So, this should mean that the CAMOS table in lowercase when taken from windows will work just fine now in linux. I am still confused as to the strategy you took above. Why are you converting stuff from windows from lowercase to uppercase; shouldn’t you be doing the opposite (simply renaming the *‘camos’* database tables to *‘CAMOS’* in linux?

My changes above will obviously require lots of time for testing.

-brady
OpenEMR

tmccormi wrote on Friday, September 14, 2012:

We converted the stuff from windows which was in UPPER case to lower case so it would work with the table name in linux.  We did NOT rename the table. We fixed the database records that use upper case.   We were concerned that changing the case of the table name would fix the old records but break new ones.  Better to make everything the same.

bradymiller wrote on Friday, September 14, 2012:

Hi Tony,

Tested the original behavior in both windows in linux. This is the way it played out:
In Windows, the CAMOS table names are in lowercase, however the entries in registry table and forms table are all in uppercase.
In Linux, the CAMOS table names are in uppercase and the entries in registry table and forms table are all in uppercase.

What I am trying to figure out is where in the windows database data is CAMOS even in lowercase??

-brady

yehster wrote on Friday, September 14, 2012:

I’m pretty sure what happens is when you create a table with upper case characters under windows, the table name is forced to be all lowercase by MySQL. 

If you do an RXNORM or SNOMED load, you’ll notice the same thing.  The scripts specify uppercase table names so if you do the import under linux you’ll get upper case tables, but under windows those same tables will have lowercase names.

bradymiller wrote on Saturday, September 15, 2012:

Hi,

Yehster appears correct.

This can’t be fixed in windows with any lower_case_table_names (note it is already set to 1 in windows by default) setting no matter what; it will always make lower case tables (at least from my reading). And since the cap CAMOS is hard-coded in openemr, there is no way to make this work in linux after the tables have been changed to small caps unless 1) set lower_case_table_names to 1 in linux  2) modify codebase/database to down-case CAMOS or 3) re-uppercase the CAMOS tables.

Since the natural progression of OpenEMR is for users to migrate from Windows to Linux as they gain expertise/comfort with the software, figured it would be best to fix this within OpenEMR rather than ask users to customize their environment or manually change table names. Note it will also be an issue for the rxnorm tables (snomed is ok and all in lowercase). So here is code to deal with this within OpenEMR (basically allows either lower-case or upper-case table names of specified tables, so will allow seamless migration from Windows to Linux without users needing to worry about this):
http://github.com/bradymiller/openemr/commits/sql-case-fix_3
(first commit is the actual feature/fix)
(second commit is fixing a bug fix to query sql errors which the audit engine was breaking)

Gonna need a lot of testing, obviously, but appears to work in both situations(use form_CAMOS as an example to illustrate):
If try to query form_CAMOS, will then try form_camos if this fails.
If try to query form_camos, will then try form_CAMOS if this fails.

-brady
OpenEMR

yehster wrote on Saturday, September 15, 2012:

Brady,

Since the natural progression of OpenEMR is for users to migrate from Windows to Linux as they gain expertise/comfort with the software, figured it would be best to fix this within OpenEMR rather than ask users to customize their environment or manually change table names.

I disagree pretty heavily with this statement.  Migrations of this kind are likely to only be done by more sophisticated users with experience in both environments.  I think it is perfectly reasonable to ask a user to make these sorts of changes as part of the process That is a small requirement to ask of a few users who want to make such a switch when compared to the risk involved in changing such a central component as sql.inc and the “lot of testing” that would be required to be confident in your fix.

I do not wish to banter with you about this as we have so many times before when we disagree. So I’m preparing myself to maintain a patch branch in my own repository reversing any such changes for my own purposes if you do end up pushing them to the official repository.  I lack confidence that changes to such a critical section of code are likely to be sufficiently tested.

bradymiller wrote on Monday, September 17, 2012:

Hi yehster,

Is this a code review? A contributor (myself) spends several hours of time actually analyzing a problem and working on a solution that is admittedly in the early stages. And your review is a two sentence armchair analysis using “banter” as an excuse to go into no depth and simply state you will not use this code… If you would mention any actual issues, I’d be glad to respond to them and modify (or kill) the code as needed.

thanks,
-brady
OpenEMR

yehster wrote on Monday, September 17, 2012:

Brady,
Based on our previous conversations, you have stated that if a feature is of any potential use to someone you favor inclusion.  My objection is based on weighing the relative costs of testing to the limited set of people who might benefit.  This is a basic position that I do not feel I can sway you from, so I’m not going to bother trying.

The complexity of this fix in such a core component is what scares me.  What happens if a query on one of the CAMOS tables fails for a reason other than the case issue?  Does it retry with the other table name, only to fail that time because the case was wrong?  Do you have test cases for this? What error gets reported in that scenario?  If there is a retry is it going to mask the real error?

I don’t like the “copy and paste” style you used in implementing your fix, but I don’t have specific technical objections to the code itself.  My “gut” feel is that this is going to cause more problems than it solves hence my decision to only provide “armchair” analysis.

bradymiller wrote on Tuesday, September 18, 2012:

Hi,

Quick thoughts on yehster’s reply: It’s generally a good idea to separate code from the developer (or code from politics) when reviewing code/project proposals. Just try to pretend it is a blinded study…

Still in analysis phase here. The current test case I am using is the following scenario:
1. OpenEMR is installed on windows
2. CAMOS is installed (so CAMOS table names are lowercase, CAMOS data entries into register and forms are uppercase and the CAMOS codebase sql queries are uppercase)
3. A CDR rule is created in Adminsitraion->Rules that uses the form_camos tables (CAMOS is stored in the CDR tables as lowercase).
4. Rxnorm is installed (so rxnorm tables names are lowercase and future codebase sql queries will be uppercase)
5. Bring it over to linux

Going through the above non-OpenEMR codebase solutions:
1) set lower_case_table_names to 1 in linux
-Still need to test to ensure this setting will work with both uppercase/lowercase CAMOS in table name
-Issue will be that it will effect other software that is using the mysql server
-Issue is that shared hosting will most likely not support this

2) re-uppercase the CAMOS and rxnorm table names (this is my initial favorite)
-Issue is that the CDR entries created in windows(since in lowercase) to query the form_camos table will break in addition to any other features that store the database table names.
-Some room for human error when converting the rxnorm tables

3) modify codebase/database to down-case CAMOS
-This would be very extensive and still wouldn’t support the rxnorm tables since it is a standardized install that uses uppercase for rxnorm.

4) Make the codebase flexible for casing of the CAMOS and rxnorm table in addition to having a method to support other tables (for example, customized forms).
-My commit above addresses this, which does the following.
a)If a sql error is thrown, then it checks the error. If it is a Table is missing error then it sees if the missing table in the error statement is one of chosen tables that are supported.
b) If it is a supported table, then it checks for existence of the opposite table (so if form_camos is missing, it will check for form_CAMOS or vice versa).
c) If the opposite table does exist, then it analyzing the original statement to ensure that all tables within the statement that are supported (for example, if have several of the CAMOS tables in one join statement) are of the same family (either uppercase or lowercase form), and that each of their opposite tables exist. (this rigid testing is done to not allow inifinite recursion)
d)If criteria in c are fulfilled, then the new statement is created with the opposite tables and run.

Regarding error reporting, note this is actually compounded by the fact that the current Execute function (with auditing) clears the error code in most environments, which I think I have fixed in below commit, which I discussed above (but still testing):
http://github.com/bradymiller/openemr/commit/e5497774ef5617ab3c79cdf4f3d4afead4768963

So, need to ensure this bug is fixed before can test/ensure that the error codes aren’t swallowed up by the requery. Some simpleton testing (with casing/dropping tables) has shown that the correct errors are shown.

-brady
OpenEMR

bradymiller wrote on Wednesday, July 10, 2013:

Hi,

Committed a much less invasive fix for this here (it leverages the already existent mechanism that “whitelists” sql table names):
http://sourceforge.net/p/openemr/discussion/202506/thread/a415cf97/?limit=25&page=1#d3c0

Do note this will not fix the CDR engine stuff brought up above (if the casing is different in a rule), however this is something a user can fix in a straightforward manner when migrating from windows to linux, if needed.

-brady
OpenEMR