Currently (D4.7, D5, D6, D7), node_delete($nid) does not remove entries matching $nid from the history table.

This does not help maintain traceability since the visited content is gone anyway, so the entries should be removed, or deletion blocked if traceability is a requirement, to avoid the possibility of erroneous links being generated by admin helper modules using that table.

A similar problem has been mentioned at #118108: Node delete does not clean up node_access table for node_access.

CommentFileSizeAuthor
#16 node_delete_from_history-318496-d6-16.patch569 bytesalbert volkman
#10 node_delete_from_history_2.patch927 bytesDeeLay
#2 node_delete_from_history.patch698 bytesDeeLay

Comments

DeeLay’s picture

DeeLay commented
Assigned: Unassigned » DeeLay
DeeLay’s picture

DeeLay commented
Status: Active » Needs review
StatusFileSize
new node_delete_from_history.patch698 bytes

Added a simple query to node_delete to remove the corresponding entry from the history table.

stewsnooze’s picture

stewsnooze
he / him
commented

subscribe

stewsnooze’s picture

stewsnooze
he / him
commented
Status: Needs review » Reviewed & tested by the community

Seems fine to me.

Anonymous’s picture

Anonymous (not verified) commented

Cascading deletes would be a nice thing to have. ;p

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Needs work

The query needs to be rewritten to use db_delete().

http://drupal.org/node/310081

sharda_ram’s picture

sharda_ram commented

subscribe

taz88ny’s picture

taz88ny commented

subscribe

DeeLay’s picture

DeeLay commented

Thanks berdir, I guess I should also change the existing queries so instead of having:

db_query('DELETE FROM {node} WHERE nid = %d', $node->nid);
db_query('DELETE FROM {node_revision} WHERE nid = %d', $node->nid);
db_query('DELETE FROM {history} WHERE nid = %d', $node->nid);

we would end up with:

db_delete('node')
->condition('nid', $node->nid)
->execute();
db_delete('node_revision')
->condition('nid', $node->nid)
->execute();
db_delete('history')
->condition('nid', $node->nid)
->execute();

DeeLay’s picture

DeeLay commented
StatusFileSize
new node_delete_from_history_2.patch927 bytes

attached new patch using db_delete

stewsnooze’s picture

stewsnooze
he / him
commented
Status: Needs work » Needs review
moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

With bonus DBTNG cleanup. Thanks.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Fixed » Patch (to be ported)

Probably needs backport to 6.x (5.x ?), since it is a bug, though minor.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Version: 7.x-dev » 6.x-dev

Setting version for backport.

albert volkman’s picture

albert volkman commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new node_delete_from_history-318496-d6-16.patch569 bytes

D6 backport.

thiagomp’s picture

thiagomp commented
Status: Needs review » Reviewed & tested by the community

Worked well on D6. I think it is fine.

The last submitted patch, 10: node_delete_from_history_2.patch, failed testing.

Status: Reviewed & tested by the community » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.