InnoDB vs. MyISAM

OpenEMR Development
61

osverdlov wrote on Thursday, May 19, 2016:

Below is list of table fields defined as TEXT with NOT NULL DEFAULT “” clause .
This definition produces warning in MySQL and the field is created simply as TEXT NOT NULL, regardless of sql_mode.

What do you think about how to convert these fields ?

history_data.exams
history_data.userarea11
history_data.userarea12

lang_custom.constant_name

layout_options.conditions
patient_data.billing_note
rule_action_item.reminder_message
procedure_providers.notes
procedure_questions.options
procedure_order.patient_instructions
procedure_order_code.diagnoses
procedure_report.report_notes
procedure_result.comments

62

mdsupport wrote on Thursday, May 19, 2016:

We should remove the DEFAULT clause. Quoting MySQL documentation

BLOB and TEXT columns cannot have DEFAULT values.

At one point this was a hard error on Windows but warning on other platforms. Since current databases would not be using the defaults, it would be safe to assume that existing code is handling this already.

Recommended way to implement defaults for BLOBs would be triggers.

63

sunsetsystems wrote on Thursday, May 19, 2016:

With NOT NULL but no default, is it an error to omit the value when inserting? But I guess even if so, the situation is not coming up and should be fine to just omit the default.

64

osverdlov wrote on Thursday, May 19, 2016:

@Rod, are you sure we always insert all the fields into patient data ? Wouldn’t it be safer to set the field as TEXT, without NOT NULL clause?

65

sunsetsystems wrote on Thursday, May 19, 2016:

That was my question, no I’m not sure.

66

osverdlov wrote on Thursday, May 19, 2016:

Yes, if the field is defined as NOT NULL and we did not supply a value, then the user will see an ugly ADODB Error message.

67

osverdlov wrote on Thursday, May 19, 2016:

Sorry, I have missed the question. Answered.

68

osverdlov wrote on Thursday, May 19, 2016:

Please check my last commit; I’m not sure these fields should be NOT NULL.

69

sunsetsystems wrote on Friday, May 20, 2016:

Looks like MySQL likes empty TEXT columns to be null, so I would remove both clauses and let null be the default. If this causes any PHP issues then fix them there, but I expect the impact will be minimal.

Rod
http://www.sunsetsystems.com/

70

sunsetsystems wrote on Friday, May 20, 2016:

See my comment in the pull request for an easy solution for the auto_increment problem.

Rod
http://www.sunsetsystems.com/

71

bradymiller wrote on Saturday, May 28, 2016:

Hi,

To take the TEXT NOT NULL issue further, should we consider changing all *TEXT NOT NULL items to *TEXT (ie. drop the NOT NULL):
http://www.open-emr.org/wiki/index.php/InnoDB_Transition#Fields_TEXT_NOT_NULL

If we were to do this, now would be the time (since changing to INNODB will be a 1 time thing where we have an opportunity to do things like this).

-brady
OpenEMR

72

osverdlov wrote on Saturday, May 28, 2016:

One place to fix would be LBF fields generator that produces TEXT NOT NULL fields definitions.

73

bradymiller wrote on Saturday, May 28, 2016:

Agreed,

I mentioned that in the above wiki link. Would there also be a way to fix the entries that have already been added by the LBF generator?(ie. would need to query in the tables that the LBF generatpr can potentially add columns too for “bad” columns and then convert them).

-brady
OpenEMR

74

bradymiller wrote on Sunday, May 29, 2016:

Hi,

Ongoing pull request for this here:

After I fixed the current translations tables to use InnoDB, the install process took more than 1 hour. I actually gave up; the lang_definitions table entries was only up to 40K of 150K rows, so guessing the install would of taken several hours…

Did some research; it appears every insert requires an i/o hit that can slow down things when do a large number of inserts. I added this mechanism to the install script(basically postpones the i/o hit to happen after all the inserts are completed rather than after each insert), which brought the installation process back to about 3 minutes or so (just a little longer, maybe twice as long, than when used the MyISAM engine):

-brady
OpenEMR

75

osverdlov wrote on Sunday, May 29, 2016:

Can you send me the output of SHOW VARIABLES command?

There are a lot of inserts in database.sql . Make sense to separate INSERTing data from database creation, and also use multi-line inserts:


INSERT INTO clinical_rules (id, pid, active_alert_flag, passive_alert_flag, cqm_flag, cqm_nqf_code, cqm_pqri_code, amc_flag, amc_code, patient_reminder_flag, access_control ) VALUES ('rule_socsec_entry', 0, 0, 0, 0, '', '', 0, '', 0, 'admin:practice'),
('rule_penicillin_allergy', 0, 0, 0, 0, '', '', 0, '', 0),
('rule_blood_pressure', 0, 0, 0, 0, '', '', 0, '', 0),
('rule_inr_measure', 0, 0, 0, 0, '', '', 0, '', 0);
76

bradymiller wrote on Sunday, May 29, 2016:

Attached the SHOW VARIABLES output.

77

bradymiller wrote on Sunday, May 29, 2016:

Regarding above settings, note it is basically default setting so Ubuntu 16.04 (I have changed nothing).

78

bradymiller wrote on Sunday, May 29, 2016:

Note the above solution did shorten the install time to only several minutes (which I am guessing would be even shorter on a non-gui linux instance).

79

osverdlov wrote on Sunday, May 29, 2016:

LBF generator in demographics actually generates TEXT NOT NULL fields which are perfectly OK as long as we update the fields during query. See addOrDeleteColumn function in edit_layout.php.

To obtain list of fields:

select column_name, column_default from information_schema.columns where data_type=‘text’ and is_nullable=“NO” and table_schema=“openemr”

80

bradymiller wrote on Sunday, May 29, 2016:

Hi,

Could this then drastically simplify the upgrade script regarding the *TEXT issues? We could simply do this on the entire database then and change them all (text, mediumtext, and longtext) to bare items. Then wouldn’t need any of the “specific” *TEXT changes in the upgrade script.

-brady
OpenEMR