sunsetsystems wrote on Monday, April 18, 2016:
OK thanks… but the primary key is not very long, and there is no secondary index for that table.
bradymiller wrote on Tuesday, April 19, 2016:
Hi,
Regarding the TEXT indexing, would the following be an example of an index that would not be supported in innodb:
-brady
OpenEMR
yehster wrote on Tuesday, April 19, 2016:
No. That’s just a text field with a standard key. No FULLTEXT index involved at all.
FULLTEXT is a “keyword” index on a field. An example where a fulltext index might be useful is with descriptions/titles of ICD 10 codes. Such an index might speed up the multi clause "like %% " queries we currently use.
Instead of building up the query with keywords like we do now, there would be a “MATCH … AGAINST” query.
yehster wrote on Tuesday, April 19, 2016:
The incompatibility with InnoDB isn’t just because of auto_increment. It’s only in very specific cases, where the key is composite and auto_increment isn’t the first part of the key.
In the tables where there is a problem, the auto_increment field has a very clear “semantic” meaning as part. (It’s being used to number related entries sequentially… (e.g. for encounter 312, here are entries 1,2,3,4…) The sequence number would still need to be generated somehow (hence the proposal of using a pre-insert trigger…) If auto_increment were moved to a separate column that was the new primary key, there would be no value for the sequence number at all, and the ordering maintained by the current approach would be lost.
mdsupport wrote on Tuesday, April 19, 2016:
I am not familiar with specific set of tables but combining mutiple logical attributes into a single physical one is a basic db design issue unrelated to table engine choice.
I love triggers but thought project stayed away from db features that are not automatically portable.
bradymiller wrote on Tuesday, April 19, 2016:
As a project, we have avoided triggers, since they are not cross compatible with other database engines.
sunsetsystems wrote on Tuesday, April 19, 2016:
Kevin, thanks for the clarification. Good to know that lbf_data is not a problem.
It seems clear that an acceptable solution will require some PHP code changes, preferably implementing common functions for inserting into the ar_activity and procedure_order_code tables (any others?)… which should be done anyway.
yehster wrote on Tuesday, April 19, 2016:
Note, I’m not proposing we use triggers for the core project. It’s a possible solution for matrix to be able to use InnoDB with the incompatible tables with minimal modification to PHP code.
Not neccessarily the ideal solution either, as wrappers/common functions as Rod mentions would be another possiblity. However, wrappers which generate the auto_increment value separately might behave differently under MyIASM and InnoDB when there is concurrency.
Auto_increment isn’t cross compatible with other databases either incidentally.
sunsetsystems wrote on Tuesday, April 19, 2016:
Regarding concurrency, last I checked PHP still does not do multi-threading (which btw is a pretty big scaling problem). Not sure if that’s what you are referring to.
tmccormi wrote on Wednesday, April 20, 2016:
Fortunately the scale that even very big OpenEMR install is likely to be is
not very big in the scheme of things…
On Apr 19, 2016 9:02 AM, “Rod Roark” sunsetsystems@users.sf.net wrote:
Regarding concurrency, last I checked PHP still does not do
multi-threading (which btw is a pretty big scaling problem). Not sure if
that’s what you are referring to.InnoDB vs. MyISAM
https://sourceforge.net/p/openemr/discussion/202506/thread/2244a470/?limit=25&page=1#eb6e/f8bc/7892/a1dc/39ea/24ed/7324Sent from sourceforge.net because you indicated interest in
OpenEMR / Discussion / DevelopersTo unsubscribe from further messages, please visit
SourceForge.net: Log In to SourceForge.net
–
Please be aware that e-mail communication can be intercepted in
transmission or misdirected. Please consider communicating any sensitive
information by telephone. The information contained in this message may be
privileged and confidential. If you are NOT the intended recipient, please
notify the sender immediately with a copy to hipaa-security@mrsb-ltd.com and
destroy this message.
osverdlov wrote on Monday, May 09, 2016:
Here’s a list of tables that could not be converted to InnoDB for now ( master OpenEMR / MySQL 5.7.12 )
ERROR 1075 (42000): Incorrect table definition; there can be only one auto column and it must be defined as a key
ar_activity
claims
procedure_answers
procedure_order_code
ERROR 1067 (42000): Invalid default value ( DATE/DATETIME field default value set to zeros).
This error can be mitigated by setting SQL_MODE to empty value.
batchcom 'msg_date_sent'
drugs 'last_notify'
drug_inventory 'last_notify'
insurance_data 'date'
openemr_postcalendar_events 'pc_eventDate'
As of “insurance_data” the only place where data is inserted to the table is in library/patient.inc . The field date is always initialized. Can we omit NOT NULL in field definition.
Same for batchcom where we can assign now() value to msg_date_sent field.
What do you think?
bradymiller wrote on Tuesday, May 10, 2016:
Hi,
Do note sql_mode in OpenEMR is now set to an empty value(this was recently added to not break in MySQL 5.7 which defaults in a bunch of sql_mode settings; Fix STRICT SQL bug. · openemr/openemr@e7aa710 · GitHub ). Will this remove the issues for the following tables then?
batchcom 'msg_date_sent'
drugs 'last_notify'
drug_inventory 'last_notify'
insurance_data 'date'
openemr_postcalendar_events 'pc_eventDate'
-brady
OpenEMR
bradymiller wrote on Tuesday, May 10, 2016:
As an aside, after getting INNODB, it would be good to start shooting for the sql_mode settings that are default in MySQL 5.7 ( ONLY_FULL_GROUP_BY, STRICT_TRANS_TABLES, NO_ZERO_IN_DATE, NO_ZERO_DATE, ERROR_FOR_DIVISION_BY_ZERO, NO_AUTO_CREATE_USER, and NO_ENGINE_SUBSTITUTION).
-brady
OpenEMR
bradymiller wrote on Tuesday, May 10, 2016:
Regarding the above “as an aside” post, placed this issue on the active projects page to track it:
http://www.open-emr.org/wiki/index.php/Active_Projects#Database
osverdlov wrote on Tuesday, May 10, 2016:
Yes, the conversion of these 5 fields was completed successfully after setting sql_mode to empty string.
osverdlov wrote on Tuesday, May 10, 2016:
I have checked for insurance_data ‘date’ and batchcom. I think we can remove default value ‘0000-00-00’ from these fields’ definitions.
osverdlov wrote on Tuesday, May 10, 2016:
To conform with sql strict mode we may require user input to pass via stricter server-side validation .
bradymiller wrote on Tuesday, May 10, 2016:
We can only dream. Just thinking about that time in the distant future where there will be no non sql strict option.
-brady
osverdlov wrote on Tuesday, May 10, 2016:
We’ve started working on the remaining 4 tables.
For example, for “claims” table: we’re going to remove AUTO_INCREMENT from version field. Then in billing.inc we’ll place INSERT inside of transaction along with SELECT to calculate/increment version field.
osverdlov wrote on Sunday, May 15, 2016:
We have submitted a pull request . We would appreciate your comments.