-
Notifications
You must be signed in to change notification settings - Fork 517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make emoncms database agnostic #106
Conversation
Nearly all backtics used are not required. The only one that will clash is 'convert' as that is a reserved MySQL reserveword [0]. This patch cleans up all backtics as those are not cross-db and propriatery to MySQL. The Ansi-SQL version of double quotes needs special support from MySQL which isn't enabled by default, so we can't use that. By using this patch, it will become easier to use other databases next to MySQL for datastorage. [0] http://dev.mysql.com/doc/mysqld-version-reference/en/mysqld-version-reference-reservedwords-5-0.html Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
processList seems to be the only column in a table that is camelCased, this looks out of place and inconsistent. This patch renames all instances and references. Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
We have a DataType enum so we don't use hardcoded values, lets make use of it. Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
The current PHP code uses mysql directly everywhere. Even when using timestore as the main database mysql still is a hard dependancy. To ease the transistion to possibly using Postgres and/or SQLite in combination with timestore, it is neccesary to abstract out mysql into an abstraction layer and make calls generic. This patch does just that and because of that adds Lib/db.php, the database abstraction layer. Queries which are standard SQL are just passed to this library. Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
With this patch we move db_check to the other db_ functions. This feels more appropiate place. Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
With the previous cleanup, it is now quite easy to expand further database support. This patch adds support for the PostgreSQL database. There is a new concept of dealing with time as an actual timestamp or the timeseries log. A new unused function is added that needs some tuning and testing, got_data(), which uses the idea of using the database's resolution for time rather then calculating our own using date_trunc. Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
This patch adds SQLite support to emoncms. There is one big caveat however that wasn't solved yet. When using SQLite as a timeseries store, it is currently not possible to drop tables. When issuing the delete command from the Webinterface, the table appears to be removed, but the actual table in the database remains. Since most users will use timestore for the timeseries log, this most likly won't be a huge issue. SQLite is also optimized for speed but none of the optimizations have been preformed such as optimizing the number of writes by caching it in memory first. Carefully tuning SQLite makes it actually possible to have it perform as if it where run from memory. One optimization is to use BEGIN TRANSACTION and END TRANSACTION around queries and while that is technically possible in db_query, this should be left to a second patch once SQLite is more stable. The second optimization is to use PRAGMA to tune parameters. Two important ones are: synchronous=OFF temp_store=MEMORY As with postgres, the current database update mechanism isn't done nor tested, only creating new databases is. Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
|
Hello Oliver, Wow! that's a lot of work you have done there! thanks for the pull request. I actually moved away from a partially implemented database abstraction layer in February this year, there's a very short note about it here: http://openenergymonitor.blogspot.co.uk/2013/02/ideas-for-improving-emoncms-framework.html I felt that using mysqli directly was nicer as it meant a new developer would not need to learn a new abstraction layer. This said Im not necessarily tied to the idea of it being mysqli based and have read about PDO with interest. http://www.phptherightway.com/#databases Could the standard php PDO be used rather than the db_query type abstraction? |
|
Great minds think a like. I did think of moving the database schema creation in there as well, but as you see, it's rather big atm and rarly needed. Saving a few bytes of RAM ;) Yeah it was a lot of work, took me 3 weeks longer then the 2 days I envisioned, which is also the answer to your PDO question. Short answer, No. Long answer, yes ;) If you look closely, SQLite is actually using PDO and yeah, we can easily transition mysql and postgres over to also use PDO. The abstraction layer will then become very sweet and simple as 90% of the functions will then be identical. Having these abstraction functions has the neat benefit of having all database functions in one place, so modifications are easier. Anyway, PDO is nice in theory, but certain things aren't supported by certain backends. As you might notice, I removed num_rows() calls, as postgres and mysql support those just fine, sqlite doesn't. There's rowCount() in PDO, but that only works for INSERT, DELETE etc statements, not SELECT. I started adding postgres support, as I have several postgres databases and having that supported was a pre for me. Once I was familiar and had things working, I added SQLite, something I never done before. Actually I first used sqlite3 and had it working all except DROP TABLE, to which I was told 'you should use PDO'. Anyhow, I figured, one step at a time ;) First get this merged, then transistion postgres and mysqli both to PDO. Once everything is PDO, great. Then we got Lib/db.php covered. Then there are also the engines that are now seperated into files. I guess those can be easily PDO'ed and maybe merged. There's only very few changes per database required, but they do exist. As for all the other files, I tried to make the SQL queries as generic as possible but some just are cursed by incompatibilities, which is why there's switch/case statements sprinkled all over the queries. |
|
Interested in learning more, as this is a big change to core it will take a while as I need to be sure that code added to core is both stable, doesnt degrade performance etc. What do you think are the advantages of using sqllite or postgres? Have you seen the latest work on integrating redis to take the load off mysql, I wonder how that fits in. Potentially that could make sqllite more applicable. I see your doing work with arm based chips is your motivation for this development to try and reduce the need for heavy components like mysql? |
|
I tried to ensure MySQL usage remained the same, some more cleanups/optimizations are possible, but I left those for after stabilization. So in theory mysql support is near identical. Some testing so squash bugs that I may have introduced (typo's etc) are of course needed, I don't have mysql. As for SQLite and Postgres; I do have and know Postgres, and it's all about choice isn't ;) With the whole oracle, myradb etc having that option is only good. I think postgres (with some tweaks to get_data) can be a little faster. In any case, postgres/mysql is just a choice thing. SQLite, especially for embedded systems can be much faster actually. More so if you use memory as cache and reduce the number of writes, which is also a boon for flash based systems. With timestore being used for the bulk of the datastorage, having the configuration bits in SQLite makes sense on top of that. It's quite interesting to test however, if SQLite (properly tuned) can outperform timestore. SQLite vs redis is an interesting one (with timestore for logging). With this changeset, adding redis support should be trivial. db_query and friends can be extended to include redis very easily, queries might not work in the same way however? Anyway, having every $sql = behind a switch case would allow you to add redis support as well. |
|
Adding in redis is almost fully implemented on the redis branch here: https://github.com/emoncms/emoncms/tree/redismetadata, you might be interested in the blog here too http://openenergymonitor.blogspot.co.uk/2013/11/improving-emoncms-performance-with_8.html I've created a branch called postgresqlite, could you send a pull request to that branch and we can develop this integration on there for a while and see how best to fit it in with recent work on redis. |
|
I will also get testing with sqlite and learn a bit more about PDO, just so that I can get my head around this a bit more. |
|
Sounds like an excellent start. |
This patch series abstracts MYSQL out of emoncms and adds postgreSQL and SQLite support.
Some things where cleaned and cleared to make it work with all 3 databases.
Not tested and probably not working:
Update older databases, only creating was tested.
Only a small dataset, was available for testing (read, 5 - 10 manual entries) and thus get_data wasn't very testable.
SQLite currently has trouble dropping tables so while it should work, it doesn't on my test database. No memory optimizations have been done, see commit message for more details.
MYSQL support, while theoretically should be still 100% identical, needs to be tested of course, since I wasn't able to.