The _is_flagged method in flag.inc returns incorrect result. As I debugged my flag action execution, I realized that the db_result returned a correct field, but when the fid value is "0" the function result is evaluated as FALSE. So I changed a little bit the result checking code.

Here is a quick fix:

function _is_flagged($content_id, $uid) {
    //return db_result(db_query("SELECT fid FROM {flag_content} WHERE fid = %d AND uid = %d AND content_id = %d", $this->fid, $uid, $content_id));
    $res = db_result(db_query("SELECT fid FROM {flag_content} WHERE fid = %d AND uid = %d AND content_id = %d", $this->fid, $uid, $content_id));
    return ($res !== FALSE);    
  }

Please, correct me if I'm wrong.

CommentFileSizeAuthor
#6 vb_dummy.sql_.txt2.54 KBmooffie

Comments

quicksketch’s picture

quicksketch
he/him
commented

Hmm, but FID should never be 0 because it is an auto-increment column. If you have a flag with FID 0, then it sounds like something is wrong with your installation. Did you upgrade from Drupal 5? I've heard a few scattered reports of the auto-increment not being added properly.

delykj’s picture

delykj commented

Yes, I upgraded from D5, but without upgrade errors.
I see the auto increment option, but only for the fcid column located in the flag_content table.

I exported my flag tables (only structure):
CREATE TABLE IF NOT EXISTS `flags` (
`fid` smallint(5) unsigned NOT NULL DEFAULT '0',
`content_type` varchar(32) DEFAULT '',
`name` varchar(32) DEFAULT '',
`title` varchar(255) DEFAULT '',
`roles` varchar(255) DEFAULT '',
`global` tinyint(4) DEFAULT '0',
`options` text,
PRIMARY KEY (`fid`),
UNIQUE KEY `name` (`name`)
)

CREATE TABLE IF NOT EXISTS `flag_content` (
`fcid` int(10) unsigned NOT NULL AUTO_INCREMENT,
`fid` smallint(5) unsigned NOT NULL DEFAULT '0',
`content_type` varchar(32) NOT NULL DEFAULT '',
`content_id` int(10) unsigned NOT NULL DEFAULT '0',
`uid` int(10) unsigned NOT NULL DEFAULT '0',
`timestamp` int(11) unsigned NOT NULL DEFAULT '0',
PRIMARY KEY (`fcid`),
UNIQUE KEY `fid_content_type_content_id_uid` (`fid`,`content_type`,`content_id`,`uid`),
KEY `fid_content_type` (`fid`,`content_type`),
KEY `content_type_cotnent_id` (`content_type`,`content_id`),
KEY `content_type_content_id` (`content_type`,`content_id`),
KEY `content_type_uid` (`content_type`,`uid`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=34 ;

quicksketch’s picture

quicksketch
he/him
commented
Title: _is_flagged method returns incorrect result » FID auto-increment missing from flags table on upgrade

Yeah, it looks like we have an upgrade bug here.

mooffie’s picture

mooffie commented
Status: Active » Fixed

Fixed, maybe.
http://drupal.org/cvs?commit=467208
http://drupal.org/cvs?commit=467210

===

Now,

I couldn't reproduce the bug.

I googled, but couldn't find a definite hint.

Since I want to resolve this issue (as part of cleaning the queue), I went with my best guess:

An AUTO INCREMENT has to be the primary key. Maybe old versions of MySQL are confused if the table already has a primary key --even if it's already composed of the column we're altering. There's something that corroborates this guess: system_update_6019() drops the primary key before converting the column to AUTO INCREMENT, even if that column is already the primary key (e.g., users.uid).

So I changed:

ALTER TABLE {flags} CHANGE COLUMN fid ... auto_increment

to:

ALTER TABLE {flags} DROP PRIMARY KEY
ALTER TABLE {flags} CHANGE COLUMN fid ... auto_increment, ADD PRIMARY KEY (fid)

This mimics what system_update_6019() does.

mooffie’s picture

mooffie commented

If the patch doesn't yet solve your problem, run this at MySQL's prompt...

ALTER TABLE flags CHANGE COLUMN fid fid int(10) unsigned NOT NULL auto_increment

...and tell us if MySQL prints any error message.

mooffie’s picture

mooffie commented
StatusFileSize
new vb_dummy.sql_.txt2.54 KB

FWIW, here's an SQL file that adds dummy "Views Bookmark" tables to the database. This lets you test the upgrade path.

Status: Fixed » Closed (fixed)

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

alexkessler’s picture

alexkessler commented

FYI
Just upgraded form 6.x-1.3 to 6.x-2.0-beta5.
The only configured flag was the default "bookmarks" flag from 5.x.
I run into the same errors with auto_increment for fid, but could fix it with some SQL (see #5).

asb’s picture

asb commented

Related:

#420250: Flag D5->D6 Update Doesn't Add Auto-increment to flags.fid Column
#629396: Data upgraded from D5, no flags.fid field not set to auto_increment
#1339884: Update from 1.3 to 2.x does not preserve auto_increment on FID column