Changed ticket status from "New" to "Fix in progress"
Action performed by Acyd Burn (Server Manager) on Mar 23rd 2007, 17:38
Bug tracker
post update not working as it should (fix completed in vcs)
when i post a post in a topic it will say for EG: Topics 3 Posts 3 but when i delete one it still say Topics 3 Posts 3 when it should say Topics 2 Posts 3 this is on the index of the forum. and on the phpBB3-CVS_2007-03-20
Comments / History
Posted by bad-dj on Mar 21st 2007, 05:43
oh i found out this but as well it updates on the Statistics bit but not the bit next to the forum if you wanted to know that.
Posted by Kellanved (Former Team Member) on Mar 23rd 2007, 16:50
Oh, well. A direct result of the post count sync fix from last time.
The scenario is the following:
delete_topics is called with auto_sync = true
delete_topics calls delete_posts to delete all posts in the topics with $auto_sync=false
delete_posts then calls delete_topics with $auto_sync=false to delete the now empty topics
The original call to delete_topics resumes, but finds that there are no topics to delete and returns without running auto sync.
Personally, I'd say this call-structure is too complex and should be simplified.
The basic fix is to ensure that delete_topics isn't called recursively (there should be no need to do so - delete_topics already knows which topics to delete):
functions_admin.php
to
and
to
The scenario is the following:
delete_topics is called with auto_sync = true
delete_topics calls delete_posts to delete all posts in the topics with $auto_sync=false
delete_posts then calls delete_topics with $auto_sync=false to delete the now empty topics
The original call to delete_topics resumes, but finds that there are no topics to delete and returns without running auto sync.
Personally, I'd say this call-structure is too complex and should be simplified.
The basic fix is to ensure that delete_topics isn't called recursively (there should be no need to do so - delete_topics already knows which topics to delete):
functions_admin.php
- Code: Select all
// Remove topics now having no posts?
if (sizeof($topic_ids))
{
$sql = 'SELECT topic_id
FROM ' . POSTS_TABLE . '
WHERE ' . $db->sql_in_set('topic_id', $topic_ids) . '
GROUP BY topic_id';
$result = $db->sql_query($sql);
while ($row = $db->sql_fetchrow($result))
{
$remove_topics[] = $row['topic_id'];
}
$db->sql_freeresult($result);
// Actually, those not within remove_topics should be removed. ;)
$remove_topics = array_diff($topic_ids, $remove_topics);
}
to
- Code: Select all
// Remove topics now having no posts?
if (sizeof($topic_ids) && $where_type !== 'topic_id')
{
$sql = 'SELECT topic_id
FROM ' . POSTS_TABLE . '
WHERE ' . $db->sql_in_set('topic_id', $topic_ids) . '
GROUP BY topic_id';
$result = $db->sql_query($sql);
while ($row = $db->sql_fetchrow($result))
{
$remove_topics[] = $row['topic_id'];
}
$db->sql_freeresult($result);
// Actually, those not within remove_topics should be removed. ;)
$remove_topics = array_diff($topic_ids, $remove_topics);
}
and
- Code: Select all
// We actually remove topics now to not be inconsistent (the delete_topics function calls this function too)
if (sizeof($remove_topics))
{
delete_topics('topic_id', $remove_topics, $auto_sync, $post_count_sync);
}
to
- Code: Select all
// We actually remove topics now to not be inconsistent (the delete_topics function calls this function too)
if (sizeof($remove_topics) && $where_type !== 'topic_id')
{
delete_topics('topic_id', $remove_topics, $auto_sync, $post_count_sync);
}
Posted by Acyd Burn (Server Manager) on Mar 23rd 2007, 17:38
Yes, i see the issue. But your fix may not work too - because it does not depend on how the function is called (if delete_posts for example is called with 'topic_id' as the where type the orphaned topics should be removed and properly synced too, the same for calling delete_topics...). A much simpler fix is currently making sure the functions do not make a recursive call by themselves. This also saves one query in delete_posts where it would grab no post id and return false).
We may revisit this mechanism in 3.2.x. Rewrites or bigger changes are now forbidden within the 3.0.x source code (at least until gold) to make sure we have a stable code base.
Much better would be a way in PHP itself to check the functions state - as if it was a recursive object.
We may revisit this mechanism in 3.2.x. Rewrites or bigger changes are now forbidden within the 3.0.x source code (at least until gold) to make sure we have a stable code base.
Much better would be a way in PHP itself to check the functions state - as if it was a recursive object.
Assigned ticket to user "Acyd Burn"
Action performed by Acyd Burn (Server Manager) on Mar 23rd 2007, 17:58
Changed ticket status from "Fix in progress" to "Fix completed in CVS"
Action performed by Acyd Burn (Server Manager) on Mar 23rd 2007, 17:58
Linked ticket with changeset: r7221
Action performed by Anonymous (I am too lazy to register) on Mar 23rd 2007, 17:59
Posted by Kellanved (Former Team Member) on Mar 23rd 2007, 22:10
Excellent point. I didn't want to propose a change to the signature of those functions (API design); but yes, that's in fact a much better solution.
