xmlsitemap replacement messed up columns after upgrade from gsitemap

If URLs were saved incorrectly, you can disable and then uninstall the module. They will be saved correctly when you re-enable it.

Fixed in CVS commit 88061.

I have same problem, several entries in sitemap are node/xxx I looked and the ones that are this way do not have entries in xmlsitemap_node table. I have changed weight, reinstalled and uninstall modules, etc.

You can wait for the next release or install the 5.x development release now.

tried the latest release of 5.x.

Still can't get see url alias in sitemap xml files.

They are correct; the problem still exists.

Here is why:

Upon installation, the module theoretically inserts every valid node into xmlsitemap_node table -- but it does not in practice.

xmlsitemap_node_enable() in xmlsitemap_node.install has a problem with the final SQL query (the one that actually performs the INSERTs):

the clause "WHERE xn.nid IS NULL AND (c.timestamp < s.last_comment_timestamp OR c.timestamp IS NULL)" in the query will pick up any node with no comments (c.timestamp IS NULL) and any node with more than one comment (c.timestamp < s.last_comment_timestamp, for >=1 of the comment entries for the node), but will obviously fail to insert any node that has exactly one comment (because c.timestamp will be equal to s.last_comment_timestamp). So the node will never be inserted into xmlsitemap_node, which will not stop it from being displayed in the sitemap but will stop it from using its proper path alias.

A tedious workaround would be to go to every node with exactly one comment, and re-save it -- except that will not work either. Because in xmlsitemap_node.module, the xmlsitemap_node_nodeapi() hook's update switch will fail silently if the node doesn't already exist in xmlsitemap_node table (the "UPDATE xmlsitemap_node... WHERE nid=$node->nid" will obviously fail if the nid doesn't exist, and there is no check for that ahead of time).

At least the first, and possibly the second, issue would need to be dealt with. I am very sorry that I didn't roll a patch up to submit with this comment -- I generally don't like to submit problem statements without a patch, myself, but I'm so swamped right now that I haven't had time to do that. I guess the absolute shortest "kludge" fix is to add another INSERT into the .install file specifically for the one-comment cases, but that seems pretty kludgy and I'm sure there's a better solution to be had by rewriting the SQL in the first place.

Also found one more small problem while I was checking out that code, but I'll submit in a separate issue, as it's not directly related.

Excellent analysis. I'll get to it right away.

Fixed in CVS commit 88212.

Kudos for the fast turnaround time! I don't get issues fixed that fast on my own module, and it's comparatively tiny :-)

Sounds like the changes didn't get into the dev release. Try getting the changes from CVS.

The update is to fix previously created nodes. Did you run xmlsitemap_node update 3?

Yes. It is designed for users who are upgrading from version 1.2, therefore it may change until version 1.3 is released.

Is the weight of the xmlsitemap_node module still 2? What is the weight of the pathauto module?

I am still unable to reproduce the problem. However, I have set up two demo sites, one for the current release and one for the development version. Please see if you can reproduce the problem there, and report your results. You will find links to the demo sites at http://xmlsitemap.darren.oh.name/.

Setting back to fixed until I hear otherwise.

You do have administrative access on the demo sites. They are fresh Drupal sites, reset every half hour. You just need to create an account. I have fixed this particular issue, so please do not set this issue to active unless you can reproduce the problem there. Feel free to open a new support request if the fix for this issue has not helped you.

There is no point in having an open issue for a problem that affects only your site. I set it to fixed because a specific problem with the module has been fixed, not because your site is fixed. If something else needs to be fixed, it's new issue.

You can enable the Generate module on the demo site and create as much content as you like. And you can enable clean URLs; I've tested it.

I was one of the people that reported the same issue. The issue I had is now fixed.

Then again, I gave a much more detailed description than "path aliases are not working." The specific issue I presented got resolved, and I now have no "node/xx" entries in my sitemap.

If this is an important fix for you, I can't imagine the 30mins to drop another installation of Drupal somewhere, throw in pathauto and this module, and generate a category and some dummy content is any more time than you've already invested saying "this still doesn't work."

So try it with a clean install (your own if Darren's is giving you so much trouble), and then report back. And if you don't have the problem with the clean install, find out what it is in your production build that isn't in the clean build that is causing that problem, and report back. It could be that another patch is necessary, but if you don't give more details no one is going to be able to take a wild goose chase upon themselves to try and fix a problem they can't see. You've been around the block here for a bit; I'm sure I don't have to tell you that without enough information to replicate a problem, the problem's very unlikely to be fixed.

I have upgraded gsitemap 1.10 to gsitemap to 1.11 and then installed xmlsitemap 1.3 in parallel. After xmlsitemap has been installed i have removed the gsitemap folder.

I saw that xmlsitemap_node.pid is filled with the timestamp value of last_changed...

I will investigate tomorrow night (US/Eastern).

I have done some manual SQL tweaks.

Executed:

UPDATE xmlsitemap_node xn, url_alias ua SET xn.pid = ua.pid WHERE ua.src = CONCAT('node/', xn.nid)

This fixed the xmlsitemap_node.PID field for all nodes that have url_aliases.

Executed:

UPDATE xmlsitemap_node xn, node n SET xn.last_changed = n.changed WHERE n.nid = xn.nid

This fixed some timestamps that have had a value of NULL.

So now - only bug in this table i have is - there are some NID's that does not have an url_alias definied. They have now the last_changed as pid. What must be the default value for this xmlsitemap_node.pid field if there is no alias? Is the value "0"?

Darren: please don't make this a support request... it's a bug in the module... the source in not clear yet, but a way to fix some bugs...

Uhhhh shi* - xmlsitemap_node.priority_override contains the value that should be in xmlsitemap_node.pid. What the heck happens here??? Are all my priority overwrites lost!???

PLACE here your SQL query that reset the xmlsitemap_node.pid to NULL of all nid's that do not have an url_alias

I've done this by hand, while the site have only 6 urls without alias...

The following will reset the priority_override values to 0 to get this fields fixed, too. However all priority_override's values set in gsitemap are now lost... how good i haven't used this feature much - yet and i only have only ~100 pages on this site. I don't like going back to my backup now, but others need to do so.

UPDATE xmlsitemap_node SET priority_override = 0

So you have now xmlsitemap_update_4... but not the source of this data deleter... my site is fixed now, but the other sites will not be upgraded until this bug is fixed... hopefully sleep well now.

Aside i cannot trust this code... is a value of "-1" in previous_comment a valid value? i have no idea what this could mean and not the time to review all code... i have not enabled comment module on this site... maybe this need to be reset to NULL, too. I have no idea...

And not to forget previously_changed have sometimes a value of NULL... looks wrong, too. I can say for sure the nodes have a node.created and node.changed value that differs... so i think there should be value in both xmlsitemap_node columns (last_changed, previously_changed).

This is about xmlsitemap_node module:

Aside, you should never do SELECT 1 FROM {system}. You cannot be save about the order... it's much saver to name the column, only a side note.

Name all columns - "*" is very unsave about the column order... this line maybe the real cause of this bugs:

INSERT INTO {xmlsitemap_node} SELECT * FROM {gsitemap}

Looks like i'm correct with this last comment. See the following mysql backup... the order of columns is different and this causes the above insert to fail.

DROP TABLE IF EXISTS `gsitemap`;
CREATE TABLE `gsitemap` (
  `nid` int(10) unsigned NOT NULL default '0',
  `last_changed` int(11) default NULL,
  `previously_changed` int(11) default NULL,
  `last_comment` int(11) default NULL,
  `previous_comment` int(11) default NULL,
  `priority_override` decimal(2,1) default NULL,
  `pid` int(11) default NULL,
  PRIMARY KEY  (`nid`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8;

Here is a patch to fix the module. This should solve the destroyed _node table for users migrating from gsitemap.

This patch is missing xmlsitemap_update_4 hook to correct currently destroyed tables. This is why i marked this patch as CNW.

Extended patch. I found the xmlsitemap_additional build from gsitemap_additional is destroyed, too. This patch fixed this, but misses the update hook too.

Thank you for your work on this, Hass. Finally we have an explanation for why some people who upgrade from gsitemap have a problem and others don't. Some updates to gsitemap added or removed columns, which would change their order. The fix is simple and will prevent the problem for those who upgrade in the future.

Fixing the problem for those who have already upgraded is not going to be easy, since we do not know the column order of the original tables. Not only that, but the priority override and URL values would be mangled if inserted into integer columns. Unless we can create an update that will work for everyone, this may have to be dealt with in support requests.

In answer to your questions about proper values for columns

• Priority override should be null to use the default value. -1 means exclude from site map.
• Previously changed should be null when link is inserted. It will be replaced with the value from last changed when the node is updated.

* Priority override should be null to use the default value. -1 means exclude from site map.
* Previously changed should be null when link is inserted. It will be replaced with the value from last changed when the node is updated.

Something seems to be wrong with this. If the value of "Priority override" is NULL the sitemap.xml only displays a blank priority - no value is visible. If i import my old data i have a default value of "0" and this displays a value of "0.0". If the field does contain NULL the SQL requests in http://drupal.org/node/194780 fails, while >=0 does not match a NULL value and therefor the WHERE does not work. This field should have a default value of "0".

Additional PID should be "0", too. If i go into a node and set "not in sitemap" the value for PID becomes NULL. The Import from gsitemap added a "0" here. You should go ONE way... aside this would solve some bugs to be consistent. I found there are many inconsistencies in the DB about NULL values and you should really take a look over your work, the current module code looks not trust worthy for me. Make it easier and don't use NULL. It would prevent you writing some complex SQL statements, while you can be save there is a value inside and you don't need to check IS NULL or 0 and such stuff. I haven't checked all, but it looks to me that code looks sometimes more difficult as it should be...

Yep i think to this update is difficult. Aside the DROP'ed gsitemap table does not allow to correct this anymore :-(. Then we should commit the latest patch now to prevent further data loss... and release a new version ASAP with the latest bugfixes.

Changed title to reflect the real bug.

Just to add.. Darren has marked my support request http://drupal.org/node/195556 as a duplicate of this issue. One difference in my specific experience is that I did not perform an upgrade from gsitemap.

Sorry, I have not had time to review the code yet. Hence my bringing the issue up as a support request and not a bug. I didn't use this topic as the title didn't appear to fit the issue I am currently experiencing. I will review my particular issue, and the XML Sitemap code and respond as soon as possible.

Just to add.. I had this problem on a site running drupal 5.5 on php5 and mysql5 server. Upgrading from gsitemap give me this problem, and also tried removing tables and reinstalling, without success. Downgrading to gsitemap 5.x-1.11 worked.

On another site running drupal 5.5 on php4 and mysql 4 server, it gave me no problems.

For an upgrade from an old gsitemap installation you NEED the patch in #46. If not your tables are broken and data IS LOST! I don't know why this patch is not already committed and a new release available. Data loss is a really critical issue!!!

Darren: PLEASE...

The order of your gsitemap columns depends on which version of gsitemap you originally installed. That's why this issue does not affect every site. The patch has not been applied because I was under the impression that it changed column values and was missing something, and I was too busy to fix it. Now I see that Hass was referring to the fact that it does not fix already-messed-up tables. Since that cannot be done automatically, this is ready to be committed.

Fixed in CVS commit 90766.

No, this only make sure a gsitemap replacement works without any harm and data loss. About fixing already broken tables you said we should only create support requests. But if this bug is release we don't need to support someone, if nothing get's broken :-).