Install file of the aggregator module doesn't give a default value for description and image fields so "add feed" fails on MySQL 5.0.18. Drupal doesn't handle this situation and shows The feed %s has been added. but this is not true. Same thing happens on Drupal 4.7.3 and 4.6.9.

I am not posting a patch because I couldn't understand why we have DEFAULT '' for required fields like title, url and not for optional description, image fields. All of these fields are NOT NULL.

CommentFileSizeAuthor
#22 i_hate_mysql_now.png281.11 KBmchrisneglia
#13 aggregator_add_feed_mysql_strict.patch.txt1.04 KBerdemkose
#12 aggregator.module_15.patch804 bytesfriedchuckles
#1 aggregator_install_mysql_missing_default.patch847 byteserdemkose
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

erdemkose’s picture

erdemkose CreditAttribution: erdemkose commented
Status: Active » Needs review
FileSize
aggregator_install_mysql_missing_default.patch847 bytes

PostgreSQL version of the same problem has been fixed. Same things happen if MySQL5 is used. I am attaching the patch this time.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

This will fix new installations, but don't we need to upgrade existing installations using a hook_update?

I see that the pgsql was accepted without an update so I leave the status at PCNR, but it seems to me that it needs work.

ChrisKennedy’s picture

ChrisKennedy CreditAttribution: ChrisKennedy commented
Version: x.y.z » 5.x-dev
Status: Needs review » Needs work

TEXT/BLOB fields should not have default values. See http://dev.mysql.com/doc/refman/5.0/en/data-type-defaults.html

erdemkose’s picture

erdemkose CreditAttribution: erdemkose commented

Yes, you are right. MySQL 5 doesn't allow default values for TEXT/BLOB fields. I don't remember why I created this issue, so I tried it again.

Now,

Drupal 5.0-beta2 creates tables successfully but some of the INSERT commands cannot be executed since no default value is defined for several columns. for example:

  db_query("INSERT INTO {blocks} (module,delta,theme,status) VALUES('user', 0, 'garland', 1)");
  db_query("INSERT INTO {blocks} (module,delta,theme,status) VALUES('user', 1, 'garland', 1)");

These queries try to create the navigation and login blocks but they fail. MySQL says:

#1364 - Field 'pages' doesn't have a default value

My issue was about aggregator so I searched the aggregator module for similar dependence:

db_query("INSERT INTO {aggregator_feed} (fid, title, url, refresh, block) VALUES (%d, '%s', '%s', %d, 5)", $edit['fid'], $edit['title'], $edit['url'], $edit['refresh']);

description and image fields are not included in the INSERT statement so this query will fail and this means you cannot aggregate any feed on your website.

I think Drupal queries must be rewritten without depending default value on TEXT/BLOB fields.

erdemkose’s picture

erdemkose CreditAttribution: erdemkose commented
Title: "Add feed" fails on MySQL 5 » MySQL 5 doesn't allow default value for TEXT/BLOB fields.

I see that watchdog table doesn't exist because it has a DEFAULT value on location which is a TEXT field.

Heine’s picture

Heine CreditAttribution: Heine commented

The watchdog table has been fixed by http://drupal.org/node/98551.

ChrisKennedy’s picture

ChrisKennedy CreditAttribution: ChrisKennedy commented
Title: MySQL 5 doesn't allow default value for TEXT/BLOB fields. » "Add feed" fails on MySQL 5 strict mode

Various errors such as this have already been reported and some have been fixed. Please keep this issue focused on aggregator.module.

kickn’s picture

kickn CreditAttribution: kickn commented

has this been fixed yet?

ChrisKennedy’s picture

ChrisKennedy CreditAttribution: ChrisKennedy commented

There hasn't been a commit from this issue.... so probably not. Feel free to test and verify this though.

allen’s picture

allen CreditAttribution: allen commented

This is the only bug im left with fixing, Have anyone found a fix or workaround, how can i get that patch to work?

rssaddict’s picture

rssaddict CreditAttribution: rssaddict commented

Are there any plans to fix this? Or a workaround?

Or should I be using another aggregation module?

friedchuckles’s picture

friedchuckles CreditAttribution: friedchuckles commented
Version: 5.x-dev » 5.1
Status: Needs work » Needs review
FileSize
aggregator.module_15.patch804 bytes

I've updated the "INSERT INTO {aggregator_feed}" statement in aggregator.module to include an empty string value for "description" and "image". I'm not sure if this is the long term solution but it fixes the "Add Feed" mysql error.

erdemkose’s picture

erdemkose CreditAttribution: erdemkose commented
Version: 5.1 » 6.x-dev
FileSize
aggregator_add_feed_mysql_strict.patch.txt1.04 KB

I created a patch for 6.x-dev. Otherwise this issue will not be fixed, I think. Whoever commits this patch, please backport it.

Aggregator in 6.x-dev throws notices but I am not fixing it because there is an issue about it. http://drupal.org/node/132369

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Needs work

I'd actually prefer to update the table definition so that we have a default value. More consistent with the rest of the code.

erdemkose’s picture

erdemkose CreditAttribution: erdemkose commented

AFAIK MySQL 5 and PostgreSQL don't allow TEXT columns to have a DEFAULT value. We discussed this earlier and didn't set DEFAULT for TEXT columns.

If we will solve this issue by changing table definitions then we have to limit the length of the column and find a way to update existing tables.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Given ChrisKennedy's point in comment #3, the options for patch are:

1) Accept the patch from #13 (http://drupal.org/files/issues/aggregator_add_feed_mysql_strict.patch.txt).

2) Change the schema of {aggregator_feed}, setting both 'description' and 'image' to VARCHAR(255). Currently both are LONGTEXT, which is total overkill. From http://dev.mysql.com/doc/refman/5.0/en/string-type-overview.html

LONGTEXT [CHARACTER SET charset_name] [COLLATE collation_name]

A TEXT column with a maximum length of 4,294,967,295 or 4GB (232 – 1) characters. The maximum effective (permitted) length of LONGTEXT columns depends on the configured maximum packet size in the client/server protocol and available memory.

With the 'image' column, this doesn't really present a problem. For 'description' it might.

Can anyone with SQL access to Drupal.org tell me the longest value of these columns in our dataset?

3) Refresh the feed on initial acquisition and store the data the first time the feed is stored. I'm not sure why Aggregator doesn't fetch and validate feeds when they are first entered. Doing so gives us a chance to tell the admin that the feed is broken so they can correct it. Even so, the table columns need to be refactored to MEDIUMTEXT or VARCHAR.

So the simplest solution is #1, since aggregator_refresh() fills in the missing data.

Anyone have data to support options #2?

hass’s picture

hass CreditAttribution: hass commented
Priority: Normal » Critical

should really be fixed for final...

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Priority: Critical » Normal
Status: Needs work » Postponed (maintainer needs more info)

http://api.drupal.org/api/function/aggregator_save_feed/6

Looks like it's already been fixed. If you can reproduce with the new code, please reset.

chx’s picture

chx CreditAttribution: chx commented
Status: Postponed (maintainer needs more info) » Fixed

If it's fixed, it's fixed.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Fixed » Closed (fixed)

Yes, this was fixed in #174348 and #148973.

mchrisneglia’s picture

mchrisneglia CreditAttribution: mchrisneglia commented
Priority: Normal » Minor
Status: Closed (fixed) » Fixed
FileSize
i_hate_mysql_now.png281.11 KB

What's going on here in some cases is an inconsistency between linux and windows with mysql 5.0+

http://bugs.mysql.com/bug.php?id=13794

The problem is that -in windows- you cannot set the longtext type with '' as a default value. It will complain when you try to create or alter the table.

The fix to make the aggregator module work, however, was for me to remove the 'not null' requirement for the longtext types and not set a default. We shouldn't have to mess with the database, but well, that's the price for using open source. Freedom isn't free!

I would prefer we run drupal on linux, but I don't have that option in my workplace.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.