Bug 7188 - case-insensitive grep of /mysql/ in C4::Context
Summary: case-insensitive grep of /mysql/ in C4::Context
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) enhancement (vote)
Target Milestone: ---
Assignee: Paul Poulain
QA Contact: Ian Walls
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 6875
  Show dependency treegraph
 
Reported: 2011-11-06 11:08 UTC by Ian Walls
Modified: 2013-12-05 19:57 UTC (History)
0 users

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7188: C4::Context::db_scheme2dbi does case-insensitive grep (1.41 KB, patch)
2011-11-06 11:22 UTC, Ian Walls
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Walls 2011-11-06 11:08:36 UTC
Porting this over from Paul's de-nesting C4 omnibus.

C4::Context does an unnecessary case-insensitive grep on the database type, which is always 'mysql', unless someone manually edits the file.  If they're doing that, they can expect buggy behaviour.
Comment 1 Ian Walls 2011-11-06 11:22:59 UTC
Created attachment 6244 [details] [review]
Bug 7188: C4::Context::db_scheme2dbi does case-insensitive grep

Checking NYTProf in C4::Context, it appears that the /i flag is highly time-consumming
As we don't support anything outside from mysql, returning directly mysql directly

If in a future version PostgresSQL works, then we will have to update this sub. But if an ORM is introduced, this code will be removed completly anyway

Testing note:
Drops execution time of the line from 300ms to 20 microseconds (in my testing).
Comment 2 Ian Walls 2011-11-06 11:41:06 UTC
This change reduces my installation from taking over 300ms to run this line, to 27 micro seconds.

In my (limited) tests, I'm also finding that using /mysql/ returns in about the same amount of time as forcing the return of "mysql", within a few micro seconds.  I don't know why, but figured I'd check.
Comment 3 Paul Poulain 2011-11-06 14:52:41 UTC
Patch pushed, please test.

QMaint, it's up to you to decide if it can be applied onto 3.6. That would be worth the price, but there's a small risk of a problem. I don't think so though, so if I were you, I would apply it.
Comment 4 Jared Camins-Esakov 2012-05-23 12:06:14 UTC
Included in 3.6 prior to 3.6.5.