SQL Server support [#1593358]

Will this module ever support SQL Server?

I have gone and looked at the module code after receiving SQL Server errors, after installing this module. I concluded that the module has custom SQL scripts written in MySQL syntax, but not SQL Server syntax. One specific error I see is "GROUP_CONCAT" function is a MySQL function and is not recognized by SQL Server.

I am in the process of building an intranet for my client and am wanting to implement this module (our current chat module is drupalchat and has a few bugs of its' own).

I am wondering if it is worth my while to re-write this module using SQL Server syntax or if someone has already begun that process and if so what is the time frame for completion and/or would you like my help? I am also considering the time allocation for altering this module vs fixing bugs in drupalchat.

Thanks in advance for any response on this.

Comment	File	Size	Author
#3	privatemsg.module.NOTGIT.patch	2.52 KB	xenphibian
#3	7.x-1.x: PHP 5.3 & MySQL 5.5, D7 1 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

xenphibian CreditAttribution: xenphibian commented 3 April 2013 at 19:21

Version:	7.x-2.x-dev	» 7.x-1.3
Component:	Miscellaneous	» Code

I have already done a port of the 7.x-1.3 release of Privatemsg to SQL Server 2008 R2 (but it should work with anything after 2008).

There are two primary issues:

1) There is MySQL specific syntax in 2 places: Use of MySQL's LIMIT keyword and GROUP_CONCAT syntax. I have resolved both. GROUP_CONCAT is specific to MySQL and is not supported by PostgresSQL, either, so there is an easy fix for that. The LIMIT syntax is simply incorrect use of the database abstraction layer (and is a very common problem with modules developed with ONLY MySQL as their concern) so there's a fix for that, as well. (In this case I have hacked it because that's the quick, ALSO WRONG, but very easy solution, it really needs to be fixed for good, but I am not a contributor.)

2) The sqlsrv driver for Drupal needs a couple of (very minor) changes that should have been there in the original, which I will be logging as bugs in just a few minutes. Specifically, the schema creation code does not escape the field names of CHECK CONSTRAINTs. This is incorrect and should be fixed.

I am considering proposing a Drupal-wide database abstraction layer addition to support GROUP_CONCAT because it is now supported in SQL Server 2012, as well. But, that's a much bigger change.

Again, I have a working copy.

I may update this as I do more testing, but I did want to let people know that this DOES work with SQL Server!

Comment #2

thorsten. CreditAttribution: thorsten. commented 4 April 2013 at 08:17

hi xenphibian,

1) ... The LIMIT syntax is simply incorrect use of the database abstraction layer (and is a very common problem with modules developed with ONLY MySQL as their concern) so there's a fix for that, as well. (In this case I have hacked it because that's the quick, ALSO WRONG, but very easy solution, it really needs to be fixed for good, but I am not a contributor.) ...

This sounds very interesting, how did you "hacked" in the "solution"?
Because a "hack" for this issuse: #1909698: PDOException: SQLSTATE[42000]: Incorrect syntax
Would be great!

Thanks thorsten

Comment #3

xenphibian CreditAttribution: xenphibian commented 4 April 2013 at 20:36

File	Size
privatemsg.module.NOTGIT.patch	2.52 KB
7.x-1.x: PHP 5.3 & MySQL 5.5, D7 1 fail

Take a look; I've included a ~~patch~~ hack file here. LOL It is intentionally NOT a git patch file, but if you can read a patch file it's pretty easy to manually fix a php file using that format.

The right answer, though, is to use the range() functions in the database abstraction layer. Sqlsrv implements these correctly, Privatemsg just doesn't use them correctly.

These changes fix both the LIMIT and the GROUP_CONCAT issues.

There appears to be one other issue that I'm currently chasing down. Something about displaying the messages (you can see when you've sent one and see when you've got a new one, but you can't actually read them, yet).

I'll reply again to this when that is working and include whatever changes are needed. This should be in the next day or so, as this is extremely important to a whole group of the sites that I'm working on (internally, for now).

I should also note that the new version of the sqlsrv driver is needed for the schema creation. There is a tiny problem with CHECK constraints with column names that are also keywords which is corrected there and prevents the schema creation errors.

If you're interested in my progress, you can drop me a line and I'll send you a link to the test site I'm working with. It is visible to the public, though you'll have to register to get access to anything other than a blank (no content) page. And, of course, being a development page ya can't count on anything working the way you'd expect.

Kev

Comment #4

david_garcia CreditAttribution: david_garcia commented 21 May 2013 at 13:58

Proposed Patch works flawlessly! Thank you very much.

To solve GROUP_CONCAT at database level and thus preventing this problem from appearing in other situations/modules I propose an implementation in:

http://drupal.org/node/1993916

You must, though, have admin level at server to be able to make the changes.

As for the LIMIT keyword, the module should be rewritten to make use of abstraction layer. Anyways, I believe that the proposed patch is good enough to make it into the module. If I find the time I will make it a GIT compatible patch and open the issue in the module itself.

David