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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DeeLay’s picture

DeeLay CreditAttribution: DeeLay commented
Assigned: Unassigned » DeeLay
DeeLay’s picture

DeeLay CreditAttribution: DeeLay commented
Status: Active » Needs review
FileSize
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
CreditAttribution: stewsnooze commented

subscribe

stewsnooze’s picture

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

Seems fine to me.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

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

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir 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 CreditAttribution: sharda_ram commented

subscribe

taz88ny’s picture

taz88ny CreditAttribution: taz88ny commented

subscribe

DeeLay’s picture

DeeLay CreditAttribution: 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 CreditAttribution: DeeLay commented
FileSize
node_delete_from_history_2.patch927 bytes

attached new patch using db_delete

stewsnooze’s picture

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

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

With bonus DBTNG cleanup. Thanks.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm 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
CreditAttribution: fgm commented
Version: 7.x-dev » 6.x-dev

Setting version for backport.

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
Status: Patch (to be ported) » Needs review
FileSize
node_delete_from_history-318496-d6-16.patch569 bytes

D6 backport.

thiagomp’s picture

thiagomp CreditAttribution: 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.