Node delete does not clean up node_access table

node.module manages the node_access table by arbitrating all writes. moving to core

So should the patched node_delete do DELETE FROM {node_access} WHERE nid=%d itself, or should it call node_access_write_grants($node, array());?

Ready to patch...

Also, how about adding a node_update_N() that retroactively cleans {node_access}?

it should delete by itself. an update function makes sense if one query can just delete all the orphans. if you have ot iterate over all nodes, then a node_access_rebuild() is in order.

Doing this in one query would require an outer join followed by deleting all the entries that are joined with null. However, I don't believe it's possible to join and delete in the same query (my SQL-fu is weak). When I need to do something similar, I first run a joined update that marks the entries as "deletable", then delete the marked fields in a second query, which still avoids iterating.

Compatibility with PostGre is another matter, but I've been able to get MySQL 4 and 5 to do this:

UPDATE {node_access} a LEFT JOIN {node} n ON a.nid=n.nid SET realm = 'temp_delete' WHERE n.nid IS NULL;
DELETE FROM {node_access} WHERE realm = 'temp_delete';

This could be called a hack, though, so I'm not sure.

DELETE FROM {node_access} WHERE nid NOT IN (SELECT nid FROM {node})?

It appears this is still a problem in D7

This should do it ... when this gets committed I'll write a backport.

#8: nodeaccess.patch queued for re-testing.

bump

Nice catch.

Committed to CVS HEAD. Thanks.

This should be ported back to D6 ... Ill slap a patch together this weekend. One question ... should I include an hook_update with the query from #6 with the patch? Thoughts?

This is a patch for d6 ...

That update function could take a long time on a huge site. Anyone concerned about that? We could consider fixing the bug but not pruning the table.

this is a patch without the update function ... I think that an update would be useful, but probably better safe than sorry

#17: node_access.patch queued for re-testing.

hmmm ... any idea why this wont apply?

Sorry to be a pest -- I haven't seen any news on this effort for awhile. No big emergency, it's easy enough to delete the orphan rows by hand in the meantime, but I'm wondering if there is a patch or solution that I have not seen?

Cheers!

#17: node_access.patch queued for re-testing.

I'm working in a system where there are half a million nodes, and because of this bug there are around two million entries in the node access table. Looking at two million rows on each page view is adding significantly to database overhead. Let me know if I can help with this.

lets try this patch

maybe this one....

The bot complains that the patch does not apply. It does apply and look fine to me, so I am RTBC-ing it.

Dear all,

Personnally, I used hook_node_grants, hook_node_access_records and to avoid having dead rows in my databse, I just implemented the hook_nodeapi to solve this problem. See below.

It prevented me touching the core and it is implemented quite easily! :p

function custom_access_nodeapi( &$node, $op, $a3 = NULL, $a4 = NULL ){
    if ( $op == 'delete' ) {
        db_query( "DELETE FROM {node_access} WHERE nid = {$node->nid};" );
    }
}

I agree it is fine to not do an update function here. Better be safe. Committed the last patch. Thanks!