The Mollom 7.x-2.0 module uses case-sensitive column names (such as contentId), where PostgreSQL expects case-insensitive column names. Installing the 2.0 version of the module fails when trying to create the index on the mollom table, and doesn't finish creating the mollom_form table, for example.

The failing query is:

CREATE INDEX "mollom_contentId_idx" ON mollom ("contentId")

The problem is that when the table was created, the contentId column got created in lower-case (because it didn't have double quotes around the column name.

A hackish patch is posted here as an idea (and not as a patch I'd actually suggest, which is why it's in the comment field and not attached as a real patch). It should at least give you an idea of the problem.

--- mollom/mollom.install 2012-04-11 23:51:13.948535733 -0500
+++ mollom/mollom.install.new 2012-04-11 23:50:56.130660827 -0500
@@ -179,8 +179,8 @@
),
),
'indexes' => array(
- 'contentId' => array('contentId'),
- 'captchaId' => array('captchaId'),
+ 'contentId' => array('contentid'),
+ 'captchaId' => array('captchaid'),
),
'primary key' => array('entity', 'id'),
'foreign keys' => array(

Files: 
CommentFileSizeAuthor
#44 mollom.postgres.d6.44.patch8.88 KBeshta
PASSED: [[SimpleTest]]: [MySQL] 6,413 pass(es).
[ View ]
#42 mollom.postgres.42.patch9.24 KBeshta
PASSED: [[SimpleTest]]: [MySQL] 7,096 pass(es).
[ View ]
#39 mollom.postgres.39.patch9.07 KBeshta
FAILED: [[SimpleTest]]: [MySQL] 7,076 pass(es), 13 fail(s), and 22 exception(s).
[ View ]
#26 mollom-postgres_breaks-1529268-25.patch58.11 KBburningdog
FAILED: [[SimpleTest]]: [MySQL] 6,728 pass(es), 70 fail(s), and 20 exception(s).
[ View ]
#24 mollom-postgres_breaks-1529268-24.patch21.32 KBburningdog
FAILED: [[SimpleTest]]: [MySQL] 6,421 pass(es), 359 fail(s), and 167 exception(s).
[ View ]
#17 mollom.install-lowercase-contentid-capchaid-2.4-20130122.diff943 bytestomww
FAILED: [[SimpleTest]]: [MySQL] 6,744 pass(es), 47 fail(s), and 38 exception(s).
[ View ]
#8 fix_issue_with_db_schema-1529268-8.patch458 bytesiSylence
PASSED: [[SimpleTest]]: [MySQL] 4,241 pass(es).
[ View ]
#6 fix_issue_with_db_schema-1529268-6.patch590 bytesiSylence
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fix_issue_with_db_schema-1529268-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Title:Mollom 7.x-2.0 does not work with PostgreSQL2.x series is incompatible with PostgreSQL
Version:7.x-2.0» 7.x-2.x-dev
Issue tags:+PostgreSQL

#1171866: Enforced fetching of fields/columns in lowercase breaks third-party integration was very recently fixed for Drupal 7 core and is not contained in any stable core release yet. I wonder whether that might fix this issue?

Could you try the latest Drupal core 7.x-dev on a temporary/testing site?

Status:Active» Postponed (maintainer needs more info)

@jaredsmith: The Drupal core fix mentioned in #1 is contained in 7.14 now. Could you try whether the module works now?

Status:Postponed (maintainer needs more info)» Active

I just tried installing Mollom 2.0 on Drupal 7.14 and PostgreSQL 9.1. Not only does it throw errors, it also breaks the site completely, since it's schema is not installed properly due to the inital error, causing errors like these:

WD php: PDOException: SQLSTATE[42P01]: Undefined table: 7 ERROR:  relation "mollom_form" does not exist [error]
LINE 1: SELECT form_id FROM mollom_form
                            ^: SELECT form_id FROM {mollom_form}; Array
(
)
in mollom_field_extra_fields() (line 2239 of /srv/www/development/d7/sites/all/modules/contrib/mollom/mollom.module).

This is the error I get when I try to install Mollom now:

% drush #dev.revealit.dk en mollom
The following extensions will be enabled: mollom
Do you really want to continue? (y/n): y
WD php: PDOException: SQLSTATE[42703]: Undefined column: 7 ERROR:  column "contentId" does not exist: CREATE INDEX "mollom_contentId_idx" ON {mollom} ("contentId"); Array                                                                                                                                                                                                                                                    [error]
(
)
in db_create_table() (line 2685 of /srv/www/development/d7/includes/database/database.inc).
Cannot modify header information - headers already sent by (output started at /usr/local/share/drush/includes/output.inc:37) bootstrap.inc:1239                                                                                                                                                                                                                                                                               [warning]
PDOException: SQLSTATE[42703]: Undefined column: 7 ERROR:  column "contentId" does not exist: CREATE INDEX "mollom_contentId_idx" ON {mollom} ("contentId"); Array
(
)
in db_create_table() (line 2685 of /srv/www/development/d7/includes/database/database.inc).
Drush command terminated abnormally due to an unrecoverable error.

I think the easiest way to solve this is to simply use lower-case for all table and column names.

Status:Active» Needs review
StatusFileSize
new590 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch fix_issue_with_db_schema-1529268-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Drupal 7.14 does not fix the issue.

Attached patch to fix issue defined by mikl in comment #6. Code changes are the same as those found at the bottom of the issue description on this page. Perhaps not the best for a permanent fix, but it allows for installation of the module.

Status:Needs review» Needs work

The last submitted patch, fix_issue_with_db_schema-1529268-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new458 bytes
PASSED: [[SimpleTest]]: [MySQL] 4,241 pass(es).
[ View ]

Sorry about that, fixed up the paths and reattached patch.

With patch fix_issue_with_db_schema-1529268-8.patch I still got errors with the indexes and the table "mollom" not created as the arrays didn't match.

I cleaned up the tables in postgresql (8.4) and enhanced the patch, see below. Please note: This needs to be verified to be sure that changing the array names has no other side effects.

diff -u mollom.install.orig mollom.install
--- mollom.install.orig Thu Jun 7 20:17:58 2012
+++ mollom.install Fri Jun 8 00:19:59 2012
@@ -100,7 +100,7 @@
'not null' => TRUE,
'default' => '',
),
- 'contentId' => array(
+ 'contentid' => array(
'description' => 'Content ID returned by Mollom.',
'type' => 'varchar',
'length' => 128,
@@ -107,7 +107,7 @@
'not null' => TRUE,
'default' => '',
),
- 'captchaId' => array(
+ 'captchaid' => array(
'description' => 'CAPTCHA ID returned by Mollom.',
'type' => 'varchar',
'length' => 128,
@@ -184,8 +184,8 @@
),
),
'indexes' => array(
- 'contentId' => array('contentId'),
- 'captchaId' => array('captchaId'),
+ 'contentId' => array('contentid'),
+ 'captchaId' => array('captchaid'),
),
'primary key' => array('entity', 'id'),
'foreign keys' => array(

@sun: I know having to modify your code to work around compatibility issues like these is annoying, but would you be willing to accept a patch that would change the Mollom module to use lowercase-only column names?

Even worse, MySQLs case-sensitivity for database names vary across platforms: http://dev.mysql.com/doc/refman/5.0/en/identifier-case-sensitivity.html

Upper case letters in table/column names: Just say no(tm).

+1 for all lower case names in databases. Avoids these unnecessary and time consuming issues.

+1 #9 solution by tomww

Sun,

I just did another test as you asked, and the problem still exists (using the 7.x-2.3 version of mollom, and Drupal 7.18).

The solution in comment 9 solved the issue for me.

Status:Needs review» Needs work

Sorry, this is still on my radar.

However, changing the table columns to be lowercase requires significant changes to the module. At this point, I think it's the only viable option, but it will take me some hours to work out a patch.

As others have reported, I don't think that the existing patch is complete, as it only changes the index definitions, but not the actual table column names. Hence, marking needs work.

As I don't think anyone besides me will be able to work out this major change, I could assign this issue to me, but OTOH, I also don't want to hold anyone's horses. ;) I hope to be able to get back to this in the next few weeks.

StatusFileSize
new943 bytes
FAILED: [[SimpleTest]]: [MySQL] 6,744 pass(es), 47 fail(s), and 38 exception(s).
[ View ]

I've updated the patch for version mollom-7.x-2.4-reworked.tar.gz

Looking only at postgres the patch fixes the creation of tables/indexes where upper case isn't allowed for columns names.
For querying postgresql doesn't care on case of columns names (except they are enclosed with quotes, e.g. "columnName"), so no further code changes needed.

--- mollom/mollom.install.orig 2013-01-22 18:01:18.000000000 +0100
+++ mollom/mollom.install 2013-02-03 10:06:04.733925637 +0100
@@ -107,14 +107,14 @@
'not null' => TRUE,
'default' => '',
),
- 'contentId' => array(
+ 'contentid' => array(
'description' => 'Content ID returned by Mollom.',
'type' => 'varchar',
'length' => 128,
'not null' => TRUE,
'default' => '',
),
- 'captchaId' => array(
+ 'captchaid' => array(
'description' => 'CAPTCHA ID returned by Mollom.',
'type' => 'varchar',
'length' => 128,
@@ -191,8 +191,8 @@
),
),
'indexes' => array(
- 'contentId' => array('contentId'),
- 'captchaId' => array('captchaId'),
+ 'contentid' => array('contentid'),
+ 'captchaid' => array('captchaid'),
),
'primary key' => array('entity', 'id'),
'foreign keys' => array(

The workarround in #19 solved it for me.

Thanks a lot - Joachim

Is there any chance that this fix will ever get adopted?

Confirming that patch at #17 worked for me on Postgres using Mollom 2.7.

What is the status of this bug? Why is it "needs work"? Are the module authors aware of the problem and the patch?

Status:Needs work» Reviewed & tested by the community

#21: I have contacted Mollom (the company), and sun, a couple of times over this, so they should be well aware of the issue.

As far as I can tell, the only reason that this haven't been merged is that sun doesn't want to.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, mollom.install-lowercase-contentid-capchaid-2.4-20130122.diff, failed testing.

Status:Needs work» Needs review
StatusFileSize
new21.32 KB
FAILED: [[SimpleTest]]: [MySQL] 6,421 pass(es), 359 fail(s), and 167 exception(s).
[ View ]

Following on from #16...

Marking critical, because enabling this module immediately breaks a drupal site (if using postgres), and drupal is unable to bootstrap. To get drupal working again, I had to apply the attached patch, enable mollom on a different postres database, export the mollom_cache table definition via drush, then import it on the broken site.

The site breaks because upon enabling mollom, drupal creates the mollom table, then tries to create 2 indices on it. The first index fails:

PDOException: SQLSTATE[42703]: Undefined column: 7 ERROR:  column "contentId" does not exist: CREATE INDEX "mollom_contentId_idx" ON {mollom} ("contentId");

This means that the mollom_form table does NOT get created, which results in drupal being unable to bootstrap, filling up watchdog with:

PDOException: SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "mollom_form" does not exist LINE 1: SELECT form_id FROM mollom_form ^: SELECT form_id FROM {mollom_form};

Site = nuked.

The attached patch simply converts all mentions of "contentId" and "captchaId" to lowercase. This allows Mollom to install and work normally.

Title:2.x series is incompatible with PostgreSQL2.x series is incompatible with PostgreSQL due to use of uppercase in column names
Priority:Normal» Critical

The last submitted patch, mollom-postgres_breaks-1529268-24.patch, failed testing.

StatusFileSize
new58.11 KB
FAILED: [[SimpleTest]]: [MySQL] 6,728 pass(es), 70 fail(s), and 20 exception(s).
[ View ]

Oh right - all of the tests will fail because they're set to look for captchaId and contentId. Eep. I've now patched the tests too, but I have no idea if the testing server will accept that. Retry.

Status:Needs review» Needs work

The last submitted patch, mollom-postgres_breaks-1529268-25.patch, failed testing.

Hmmm. Looks like the Mollom API wants a case-sensitive value of captchaid - here's a failing test:

Response: 400 Bad Request
  code = '400'
  message = 'Missing resource ID'
Error 400: Bad Request (50.23.148.2)
Request: POST http://dev.mollom.com/v1/feedback
  captchaid = 'TESTdhheuk84sp2jzm'
  reason = 'spam'

So all integration with the Mollom API is case-sensitive while all database stuff is lowercase. That's a bit, well, messy. Is this still a good direction to go in?

@sun hasn't been writing since January 2013.

I think Mollom doesn't want PostgreSQL Drupal customers. As infuriating and frustrating as it is, It's time to acknowledge that. Is there another plugin that can handle comment spam?

Some good news - #1622982: PostgreSQL auto-converts column names into lowercase now passes, which means it can be backported to d7, which automatically will solve this issue. So let's hope we get some feedback over there soon!

Can we possibly get a patch for the module before the backport? I see #8 works :) Curiously I have an old project running postgres with Mollom 7.x-2.2 and we never had an issue with it

Since the Mollom module doesn't have an uninstall feature, it looks like I have to manually remove tables in the database in order to reinstall and patch. Simply patching the install file and enabling the module results in a dead website.

Any suggestions before I go butchering up my database manually? Something I'd rather not do but at this point I'm drowning in spam. I miss having Mollom on the site.

I just wanted to stop by and say that the patch in #8 worked like a charm for me (Drupal 7.23, PostgreSQL 9.2.4 running on Arch Linux). I did however have to perform the following:

$ psql <dbname> <username>
<dbname>=>drop table mollom;
DROP TABLE
<dbname>=>\q

It was as easy as that...once that was done i was able to run the patch on the install file and enable & configure mollom without issues.

just my 2 cents :)

@smscotten the problem actually comes from core I believe with the uninstall issue you are having. If you enable a module and it throws an error for the db table, it will not be removed by uninstalling the module (this is not the same as disabling). If there is an error in the db table you have to manually drop it as per @bsisco's comment.

If a module provides a db table and it doesn't throw an error on installation, you can easily get rid of it with:

drush dis module
drush pm-uninstall module

Drupal 7 does not require uninstall hooks to remove the db tables.

@bsisco @acbramley thanks. Dropping the table did the job and I was pleasantly surprised to learn that I didn't lose any of my config info (stored in the variable table, I assume.) I'm back in the land of Mollom on my PGSQL D7 install.

Issue summary:View changes

Currently having this same issue with 7-2.9, Drupal 7.26, and PostgreSQL.

I prefer using PSQL and this is diasspointing it will probably never be integrated into the module.

Looks like it's time to patch the files ...

I can also confirm that the patch in #8 fixed the issue for me.

Going to take a stab at re-rolling this today and fixing up the tests as well as a review. Just wanted to chime in and let you all know that this hasn't fallen off the Mollom radar.

eshta
Mollom engineering

StatusFileSize
new9.07 KB
FAILED: [[SimpleTest]]: [MySQL] 7,076 pass(es), 13 fail(s), and 22 exception(s).
[ View ]

So what I tried to do here was to update all of the column names that are mixed case to underscore separated column names. This should alleviate the pain with various case sensitivity levels across databases and platforms. Then I updated the crud functions to allow for the changed names while still maintaining the status quo in the normal variable names that are passed around. The hope is that this limits the affected code and therefore the likelihood for error. I'd love a review, especially given that this falls kind of in between the two main approaches I see i this thread.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 39: mollom.postgres.39.patch, failed testing.

StatusFileSize
new9.24 KB
PASSED: [[SimpleTest]]: [MySQL] 7,096 pass(es).
[ View ]

Here's another attempt with corrections for the fact that the stdClass is passed by reference.... hopefully past tests ;-)

Status:Needs work» Needs review

Version:7.x-2.x-dev» 6.x-2.x-dev
StatusFileSize
new8.88 KB
PASSED: [[SimpleTest]]: [MySQL] 6,413 pass(es).
[ View ]

Committed patch for D7.
Attaching a backport to D6 now.

Status:Needs review» Fixed

So excited to get this one in! This allows folks to come over to the 2.x line.

  • Commit 39d405c on 7.x-2.x, fbajs by eshta: Issue #1529268 by eshta: 2.x series is incompatible with PostgresSQL due...