@anonymous-piwik-user opened this Issue on May 6th 2010

I have changed the source(core) to abstract alot of the mysql specific code to different providers. This has allowed me to support Microsoft Sql Server 2008. I have created a patch that would allow for the co-existence of mssql and mysql. This is my first time to ever do php, I have done a lot of C#, asp.net, database and just coding in general. So if you see things Im doing incorrectly I would greatly appreciate the feedback or pointers.

In the attached patch, I have config files for mysql_pdo, mysqli & mssql.

I used the CTP versions of the Microsoft 2.0 drivers for php listed below:. Youd have to download the ones for your specific env. http://www.microsoft.com/downloads/details.aspx?FamilyID=df4d9cc9-459c-4d75-a503-ae3fceb85860&displaylang=en

In my php.ini I had this:

extension=php_pdo_sqlsrv_53_ts_vc6.dll
extension=php_sqlsrv_53_ts_vc6.dll

;didnt turn these on
[sqlsrv]
;sqlsrv.LogSubsystems=-1
;sqlsrv.LogSeverity=-1
;sqlsrv.WarningsReturnAsErrors=0

Keywords: wishlist

@anonymous-piwik-user commented on May 6th 2010

Attachment: Patch for Abstracting Data Access & Support for Mssql
piwik_abs_dataaccess_mssql.patch

@halfdan commented on May 8th 2010 Member

This looks very promising! If this patch gets applied to the core I'd like to finally port Piwik to PgSQL. This patch could also make it easy to port Piwik to NoSQL databases like InfiniDB or Cassandra.

@robocoder commented on May 8th 2010 Contributor

Centralizing all the SQL in the adapters may work for core but I suspect it's the wrong design approach for plugins.

The schema and DDL queries are only of interest during installation and updates. Rather than bloating the adapters, we could put these into separate classes. This decoupling would allow MYSQLI and PDO_MYSQL to reference a single schema class (and in the future allow schemas that are storage engine specific).

@halfdan commented on May 9th 2010 Member

vipsoft: Can you give a small example of your ideas? I'm currently trying to evaluate possible implementations for a NoSQL (Cassandra) backend.

@mattab commented on May 9th 2010 Owner

halfdan, noSQL would be very hard to implement (see discussion in http://lists.piwik.org/pipermail/piwik-hackers/2010-February/000834.html )

A possible 'cheaper' to implement option would be InfiniDB which is a mysql storage engine. We have contact with the InfiniDB people. One of the limitation is the absence of BLOB data field in Infinidb at the moment.

@anonymous-piwik-user commented on May 10th 2010

I agree vipsoft. Since this was the first time I have used php, I don't know what all is really possible with the language. But at least it was a start to move the code out of the core. Really to me the correct pattern would be a true repository [http://martinfowler.com/eaaCatalog/repository.html], that separates the true domain from the data access. Right now all the code has been "moved" to the these "Pdo" classes each one implementing a very large interface. Really it's doesn't adhere to Single Responsibility Principle. It's too much responsibility in this one PDO class.

I am accustom to using generics and repositories. For example, if I have a Customer Class that is considered what I call an "aggregate/domain" class, I would typically create an ICustomerRepository interface, which is what the domain class uses to communicate with when it needs to "get/update/insert/delete" data. Then each provider is responsible for implementing the interface, So if you have Microsoft Sql Server provider it would implement the ICustomerRepository Interface in a class called MssqlCustomerRepository. This would provide the implementation for TSQL Microsoft SQL Server.
With this type of pattern or design, your client code (say the core or plugins) don't care about the implementation of the data access (encapsulation/information hiding). So for example if you wanted to implement your "blob" type with a file system and your tables as xml files you could, that may not be that fast but at least you can hide all those details in each concrete implementation.

This also allows for dependency injection so you can do in memory databases for fast testing.

@robocoder commented on May 12th 2010 Contributor

(In [2167]) refs #1335 - move adapters in core/Db to core/Db/Adapter, mirroring libs/Zend/Db/Adapter

@robocoder commented on May 19th 2010 Contributor

It looks like the patch only centralizes a subset of Piwik's SQ, ie the queries which aren't portable to MSSQL. To support other databases, we'll have to increase the scope of change.

Expanding further on my comment:2, I'm reorganizing core/Db:

  • added core/Db/Adapters in [2167]
  • will refactor Piwik.php into core/Db/Schema, and provide helper methods for plugins

I looked at the Repository pattern and because it centralizes the SQL, it isn't a good fit for Piwik's plugin architecture. We had a brainstorming session and came up with some ideas. I'm going to flesh these out a bit more to see which one (or two) makes more sense in the long term.

@robocoder commented on May 20th 2010 Contributor

Note: re core/Db/Schema. For PostgreSQL and MSSQL, there's a one-to-one relationship between the adapter and schema. For MySQL, there's a many-to-many relationship between the adapters (MYSQLI and PDO_MYSQL) and storage-engine specific schemas.

@anonymous-piwik-user commented on May 20th 2010

Replying to vipsoft:

It looks like the patch only centralizes a subset of Piwik's SQ, ie the queries which aren't portable to MSSQL. To support other databases, we'll have to increase the scope of change.

Expanding further on my comment:2, I'm reorganizing core/Db:

  • added core/Db/Adapters in [2167]
  • will refactor Piwik.php into core/Db/Schema, and provide helper methods for plugins

I looked at the Repository pattern and because it centralizes the SQL, it isn't a good fit for Piwik's plugin architecture. We had a brainstorming session and came up with some ideas. I'm going to flesh these out a bit more to see which one (or two) makes more sense in the long term.

Sorry, I don't understand what you mean when you say, "which aren't portable"? I haven't come across any of the mysql that I couldn't do with TSQL. Really with the mess of the non-standard Sql strung out and scattered everywhere thru the code, it was very hard just to get the part I did centralized. If you feel it was done poorly, then by all means just scrap the patch and continue on with your feature of "mysql only support".

What about separation of concerns? Persistence is infrastructure, not part of your domain (or shouldn't be). Currently the persistence is so tightly coupled with everything, it's about impossible to support any other type of storage engine. All I did was try to at least pull some of it to a "central" place, I by no means implemented a repository.

To summarize the Repository Pattern as just "centralizing sql" is somewhat marginalizing the pattern. The Repositories should be a facade between your application/domain and your persistence storage. You application should not have that much knowledge about "how" the data is stored. You could have a database, or you could have many different databases. You could use relational database, or an object database. You could have an in memory database (like for testing), or a singleton containing a list of in memory items. You could have a REST layer, or a set of SOA services, or a file system, or an in memory cache. You can have anything your only limitation is that the Repository should be able to act like a Collection to your domain. Perhaps reading the book Domain Driven Design by Eric Evans (or at least the Repository chapter) will give better insight.

I apologize for wasting anyone's time.

@robocoder commented on May 20th 2010 Contributor

We all appreciate the effort you put into this. I can be especially terse when posting from my iPhone, so I apologize if my comments were dismissive of your hard work.

@mattab commented on May 24th 2010 Owner

jwisener, please see the new ticket at: #1368

@robocoder commented on June 6th 2010 Contributor

(In [2278]) refs #1335, refs #1368 - check in Jason's Piwik_Db_Adapter_Pdo_Mssql class (excluding the schema and query methods being refactored elsewhere; edited to conform to coding style guidelines; note: a lot of the logic in getConnection() looks like it should be moved upstream to ZF (2.0 CTP1 related?) especially if CTP2 introduces breaking changes to the DSN

@mattab commented on August 29th 2014 Owner

see #6029

This Issue was closed on June 7th 2010
Powered by GitHub Issue Mirror