I updated menu_node from 1.3 to 1.4. Before I applied the update I looked at the menu_node table. There are 459 rows. 458 are distinct. I made no alteration to the table.

The update didn't apply fully because of the duplicate in the menu_node table.

Here's the the relevant part of the output...

Update #6002

* ALTER TABLE {menu_node} DROP PRIMARY KEY
* Failed: ALTER TABLE {menu_node} DROP INDEX nid
* Failed: ALTER TABLE {menu_node} ADD PRIMARY KEY (mlid)
* ALTER TABLE {menu_node} ADD INDEX nid (nid)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agentrickard’s picture

What kind of DB are you using? Did you use the patch or the released code version?

agentrickard’s picture

Status: Active » Postponed (maintainer needs more info)
FileSize
49.48 KB

Works fine on my test box.

agentrickard’s picture

What were the non-unique rows in your table? Did update_6001 not clear them out?

matglas86’s picture

Title: the mlid patch doesn't fully apply » The mlid patch doesn't fully apply to existing duplicates
FileSize
894 bytes

I was having this same problem. Here is how it fixed the duplicate records that already where in the table. This query selects all the duplicate nodes that are referenced. Thats only 'node/%'. The patch should be rolled against version 1.4.

/**
 * Erase bad records from the {menu_node} table.
 */
function menu_node_update_6001() {
  $return = array();
  $result = db_query("SELECT mn.mlid, mn.nid FROM {menu_node} mn LEFT JOIN {node} n ON mn.nid = n.nid LEFT JOIN {menu_links} ml ON ml.mlid = mn.mlid WHERE n.nid IS NULL OR ml.mlid IS NULL");
  while ($data = db_fetch_object($result)) {
    $return[] = update_sql("DELETE FROM {menu_node} WHERE mlid = $data->mlid AND nid = $data->nid");
  }
  
  //Dupblicate records for nodes.
  $result = db_query("SELECT mn.mlid, mn.nid FROM {menu_node} mn LEFT JOIN (SELECT mlid, count(mlid) as count FROM {menu_node} GROUP BY mlid ORDER BY count desc) as mcount ON mn.mlid = mcount.mlid LEFT JOIN {menu_links} ml ON mn.mlid = ml.mlid WHERE count > 1 AND mn.nid NOT LIKE SUBSTRING( ml.link_path FROM 6) ORDER BY mn.mlid;");
  while ($data = db_fetch_object($result)) {
    $return[] = update_sql("DELETE FROM {menu_node} WHERE mlid = $data->mlid AND nid = $data->nid");
  }
  return $return;
}
matglas86’s picture

Status: Postponed (maintainer needs more info) » Needs review

Forgot to change the status. Please review my patch in the previous post.

agentrickard’s picture

Status: Needs review » Postponed (maintainer needs more info)

I still need my initial question answered. It is legitimate to have two MLIDs pointing to the same NID, so what are you considering the "duplicate rows"?

bstoppel’s picture

I had the same mlid but it was pointing to two different nids. I only had one duplicate though.

agentrickard’s picture

Status: Postponed (maintainer needs more info) » Needs work

Thanks. I really don't like the query in #4 and wonder if it can be made a little nicer. I fear it won't run on all platforms.

matglas86’s picture

Ok I think I did not get a question out of the initial post. A MLID should always point to one node if its to a node. Basically a MLID always has 1 destination. But your question is the other way round. Two MLID's to one NID. That is a no problem and should be able. Two menu items (e.a. MLID records) can both have the same end destination.

But my problem was different and should be fixed with the MLID as a primary key. When items where added is check against the primary key combination of MLID and NID. This created duplicate MLID's in my menu_node table. The module update fixed the primary key problem but not the duplicate MLID's that where already in my menu_node table because of the behavior of the previous version.

With the code I posted above I tried to fix that and clean out the duplicate MLID's in the table. When I leave them in there the change to a single primary key could not be done. It would give a error.

@agentrickard: I know that the query is not that nice. I tried to keep it simple but had to do a check for count. This is just a start.

I hope this explains my approach above.

agentrickard’s picture

Yes, thanks. The question was in #3.

bstoppel’s picture

I just realized that I missed the questions in #1.

The database is MySQL 5.0.51a . I used the release code.

agentrickard’s picture

:-) What I'd really like is someone to copy a sample of "duplicate rows",which I suspect looks like so:

MLID | NID
123 | 20
123 | 21

In this case, I think we have to delete all the dupes and rebuild from {menu_links}.

matglas86’s picture

FileSize
38.64 KB

Here is a example of the duplicates I had.

agentrickard’s picture

I think we run that query, then loop through and delete any entries where the NID doesn't match the expected path.

bstoppel’s picture

Not sure this will help or not, but here are a few rows from my {menu_node} table

mlid | nid
3649 | 29
3649 | 35 <- this was the offending row in my case. I deleted it.
815 | 29

The nid 29 is in two different menus, which is desired.

agentrickard’s picture

@bstoppel

What do you get when you run the query shown here --> http://drupal.org/files/issues/duplicates_0.png

matglas86’s picture

@agentrickard What do you mean by #14 "I think we run that query"? Do you mean that the query is already in the module? If it does somehow it does not accomplish the desired effect.

agentrickard’s picture

No, I mean "I think the solution is to run the query and loop through the results."

matglas86’s picture

When you loop through the results you will be deleting and and regenerating references right?

agentrickard’s picture

Yes.

matglas86’s picture

So it would look something like this. I didnt test the function yet the results of the query are good though.

/**
 * Erase bad records from the {menu_node} table.
 */
function menu_node_update_6001() {
  $return = array();
  $result = db_query("SELECT mn.mlid, mn.nid FROM {menu_node} mn LEFT JOIN {node} n ON mn.nid = n.nid LEFT JOIN {menu_links} ml ON ml.mlid = mn.mlid WHERE n.nid IS NULL OR ml.mlid IS NULL");
  while ($data = db_fetch_object($result)) {
    $return[] = update_sql("DELETE FROM {menu_node} WHERE mlid = $data->mlid AND nid = $data->nid");
  }

  // This is the new part.
  $result = db_query(
    "SELECT mn.mlid, mn.nid, ml.link_path FROM {menu_node} mn 
    LEFT JOIN (SELECT mlid, count(mlid) AS count FROM {menu_node} GROUP BY mlid) mcount ON mn.mlid = mcount.mlid 
    LEFT JOIN {menu_links} ml ON mn.mlid = ml.mlid WHERE mcount.count > 1 AND ml.router_path LIKE 'node/%%'");
  // Walk through every row and test them based on nid.
  while ($data = db_fetch_object($result)) {
    $nid = str_replace('node/', '', $data->link_path);
    $check = (bool) $nid == $data->nid;
    // If check failed we delete the row with this combination.
    if (!$check) {
      $return[] = update_sql("DELETE FROM {menu_node} WHERE mlid = $data->mlid AND nid = $data->nid");
    }
  }
  
  return $return;
}
agentrickard’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

Correct, though we actually have to duplicate the update functions for things to work properly, but that won't cause any harm.

If this tests OK, we can roll a new release.

matglas86’s picture

Just a minor thing to keep the code clean, remove '// This is the new part.'
Hope I'm not a pain in the ass. :-)

agentrickard’s picture

Status: Needs review » Fixed

Committed and released. Thanks for sticking with this one.

matglas86’s picture

Its no problem. I'm glad that I could help. All the little things make Drupal the best. :)

Status: Fixed » Closed (fixed)

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