node_access_rebuild incorrectly grants everyone access

Subscribing to this issue.

Looks okay, but needs a comment in the code.

this bug is a frequent source of pain for og sites. they file many mysterious bug reports.

*huge* pain! I spent half of today scratching my head wondering if I totally misunderstood how node access works...

In my case, there was a broken row in the node table causing node_load() to return false for this row (which is good), and thus the trouble with node access began.

I would ask whether node_access_write_grants() (the real culprit) and node_access_acquire_grants() should do some error checking as well.

I am bumping this to critical and committed a commented version to 5.x. It was tested by dww too.

Attached is the modified version for 6.x.

Performance is another big problem with node_access_rebuilt - when there are many nodes.

I came up with this optimization patch for sites where OG is the only access control module. The idea is to process nodes that do not belong to groups in a single query. This has been tested and could be committed to both 5.x and 6.x.

Committed Niel's patch from #5 to Drupal 6.x.

@dkruglyak: if you have 'another big problem', please open a new issue. It also looks highly unlikely that og tailored (even og named!) functions end up in Drupal core, if they have no general usefulness.

This patch fixes a showstopper for upgrading 4.7 sites, like mine. Upgrade script times out on node rebuild.

At the very least I have it posted here for review. Performance of node access rebuild is a core issue and is mostly exercised by OG.

However showstopper or critical it is, *this* issue deals with a permission bug, not a performance problem. Because the issue you mention is important and others scratch that itch too, there are issues such as http://drupal.org/node/144337 for this very reason.

the node_access rebuild was also a show stopper for my large Drupal / og site. The .install called three of these in a row, timing out every time. I had to rip the .install file apart with a text editor and incrementally run the other sql updates with a single node_access rebuild. Luckily that was no problem for me but for a new user or someone who can barely manage a drupal install, that would be a total show stopper to their drupal 5 upgrade.

I'm agreed that there is a seperate issue that is fixing this for Drupal 6+, however there is no feasible way to backport that approach to drupal 5 and the patch for this issue is now fixing that problem. Also if a single node_access_rebuild() times out then the batching is not going to fix anything, where the performance enhancements here will.

So I think that maybe the title for tihs issue should be changed to mention performance as well, as it addresses this important problem and people might be searching for that?

Until 5.2 comes out finding this patch may be someone's only way of getting a single node_access_rebuild() to run on a large 4.7 og site.

It is very much understandable and unfortunate that you have performance problems, but this issue was not about performance a single bit before dkruglyak posted his unrelated issue and patch. I would appreciate if you would move on to a new issue, and not hijack existing unrelated issues. That would save us all a (now considerable) bit of time. A properly targeted issue with a relevant title on the actual issue content would just as well enable people to find this interim solution, if they are happy with running on modified Drupal installs.

Also your concerns about a batched node access rebuilder are better shared in the relevant issue.

Please only reply here if you have comments on the "node_access_rebuild incorrectly grants everyone access" problem discussed in this issue.

Created a separate issue: http://drupal.org/node/163862

This should be done in 6.x as well...