[drupal-devel] [bug] Twin problems with comments
Junyor
drupal-devel at drupal.org
Fri Mar 25 13:37:06 UTC 2005
Issue status update for http://drupal.org/node/11366
Project: Drupal
Version: cvs
Component: database system
Category: bug reports
Priority: critical
Assigned to: Junyor
Reported by: pennywit
Updated by: Junyor
Status: active
The problem I mentioned in my last comment is
http://drupal.org/node/19432.
I've looked at the change that Dries did and I honestly can't figure
out why the change was made. I'd be happy to provide a patch for 4.5.x
if I could figure out the problem. If anything, I think the query Dries
is doing is returning too many matches, not the query I was doing.
Junyor
Previous comments:
------------------------------------------------------------------------
October 7, 2004 - 15:23 : pennywit
I have two problems with my upgrade at http://www.pennywit.com. First
is that a number of comments have the body turn to the number 3. It
looks like this happens when one of my users tries to edit his
comments. The second is that in any nodes submitted before I upgraded,
I don't see a count of the comments already submitted. I'm echoing this
to the support forum ...
------------------------------------------------------------------------
October 7, 2004 - 16:01 : pennywit
It says this has been fixed ... what do I need to do on my side?
--|PW|--
------------------------------------------------------------------------
October 8, 2004 - 09:11 : Bèr Kessels
http://drupal.org/node/11316`was the bug. Update commment.module will
fix this problem.
------------------------------------------------------------------------
October 17, 2004 - 13:04 : kps
This problem remains (in 4.5.0-rc 1/2 hour before this post):
"
... any nodes submitted before I upgraded, I don't see a count of the
comments already submitted.
"
The {node_comment_statistics} table is evidently not initialized
correctly.
Justification for "critical": Since most people who install 4.5.0 will
be users of older releases, upgrading really ought to work before the
release.
------------------------------------------------------------------------
October 17, 2004 - 13:47 : kps
Attachment: http://drupal.org/files/issues/updc.php (1.26 KB)
Attached, raw and as-is, the script I used to initialize the
{node_comment_statistics} table.
------------------------------------------------------------------------
October 20, 2004 - 18:12 : kps
Attachment: http://drupal.org/files/issues/updatecommentcount.php (907 bytes)
My above attachment (updc.php) is broken. This one is... better.
Visiting this page should at least populate the node_comment_statistics
table with something not entirely unreasonable.
------------------------------------------------------------------------
October 25, 2004 - 08:39 : Junyor
kps is right, the update won't correctly populate the table. Here's
(part of) update_105:
"INSERT INTO {node_comment_statistics} (nid, cid,
last_comment_timestamp, last_comment_name, last_comment_uid,
comment_count) SELECT n.nid, 0, n.created, NULL, n.uid, 0 FROM {node}
n"
So, it's setting the comment_timestamp = node creation time, forgetting
the comment_creator, using the userid that created the node as the
last_comment_uid, and setting the comment count to 0.
update_105 needs to be redone and I'd recommend a new update be created
for 4.5.1 that will correctly initialize the node_comment_statistics
table.
------------------------------------------------------------------------
November 12, 2004 - 10:02 : Junyor
OK, here's two patches to solve the problem.
Issues fixed:
- Fixes node_comment_statistics table prefixing for PostGreSQL
- Drops CID column from node_comment_statistics and supporting code in
comment.module
- Initializes comments table with usernames (needed by
node_comment_statistics table)
- Correctly initializing node_comments_statistics table
- Removed duplicate query for last_comment_name in
node_comment_statistics query in comment.module
Needs checking:
- I couldn't test this with PostGreSQL
- I'm no database expert, so the updates.inc changes need to be gone
over with a fine-toothed comb
- Do we need any special handling for comments with no 'name', i.e.
truly anonymous comments
These patches are for the DRUPAL-4-5-0 branch.
------------------------------------------------------------------------
November 12, 2004 - 13:01 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats.patch (7.89 KB)
------------------------------------------------------------------------
November 12, 2004 - 13:01 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats2.patch (3.44 KB)
Actually attaching patches. :)
------------------------------------------------------------------------
November 14, 2004 - 21:10 : Dries
I'm not 100% sure but isn't the name field of the comments table
supposed to be NULL, unless the comment is an /anonymous comment/? Is
it really required to initialize the name field? What happens if you
don't? I just checked drupal.org's database and only /post Drupal
4.5.0 comments/ have the registered user's name in the comment table.
I'm a little nervous about committing database changes to stable
branches (DRUPAL-4-5) so please make sure this patch is well-tested.
The patches don't apply against HEAD but I can port them once we/you
ironed out the last glitches.
Otherwise these patches look fine. Good job.
------------------------------------------------------------------------
November 14, 2004 - 23:41 : Junyor
I don't know if it's necessary, but I'm just making sure it's
consistent. Maybe the author of the node_comment_statistics patch
could comment? I'm also concerned about having the name information
for registered users outside of the users table.
I've tested this on a test site and I'll try testing on my production
site once we have this worked out.
------------------------------------------------------------------------
November 15, 2004 - 10:45 : Dries
I don't know whether the original author (ccourtne) is still around but
it should be easy enough to test whether it is necessary or not. From
what I've seen, it isn't necessary. In fact, your current patch might
break things: like, what happens when somone changes his or her
username? AFAIK, the old username would be shown in the comments.
------------------------------------------------------------------------
November 15, 2004 - 11:03 : Junyor
OK, I'll edit that bit and resubmit. You'd like this for HEAD only?
------------------------------------------------------------------------
November 15, 2004 - 18:24 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats-2.patch (7.24 KB)
New patch for database files.
------------------------------------------------------------------------
November 15, 2004 - 18:25 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats2-2.patch (3.46 KB)
New patch for comment.module.
------------------------------------------------------------------------
November 22, 2004 - 11:31 : Junyor
Attachment: http://drupal.org/files/issues/node-comment-statistics_head.patch (7.26 KB)
Patches for head.
------------------------------------------------------------------------
November 22, 2004 - 11:31 : Junyor
Attachment: http://drupal.org/files/issues/node-comment-statistics_head2.patch (3.45 KB)
------------------------------------------------------------------------
November 23, 2004 - 23:24 : Dries
I'd like to move forward with this patch and include it in Drupal 4.5.1.
I can't reproduce this problem (it seems) so it would be much
appreciated if those who can, can test it.
------------------------------------------------------------------------
November 29, 2004 - 00:42 : Junyor
It applied and works well on my 4.5.0 site. A lot of nodes were listed
as not having comments before, but they're working now. Dries alerted
me to a drupal-support message regarding the patch and I'll look into
that tomorrow.
------------------------------------------------------------------------
December 8, 2004 - 06:12 : crw
After applying the patch, approving new comments still does not update
the node_comment_statistics table, therefore the comment_count field is
off and the number of new comments does not show up on the main page of
my site.
I'm currently troubleshooting this and will report my findings here.
------------------------------------------------------------------------
December 8, 2004 - 06:13 : crw
I should point out that I applied the following patch:
node-comment-statistics_head2.patch
And the problem described above still exists.
------------------------------------------------------------------------
December 8, 2004 - 06:25 : crw
Ok, headway.
Looks like submitting a comment triggers an updating of
node_comment_statistics, but editing a comment to publish/unpublish
does not. Both should trigger the update, so I just need to find out
why it isn't working for the 'update' case.
------------------------------------------------------------------------
December 8, 2004 - 07:22 : crw
Ok, I found the problem and came up with a solution. I've been manually
approving comments from anonymous users via the admin interface.
comment_admin_edit() calls comment_save(), but neither calls
_comment_update_node_statistics. I inserted the following two lines
around line 952 of comment.module after the call to comment_save():
$nid = db_result(db_query('SELECT nid FROM {comments} WHERE cid =
%d',$edit['cid']));
_comment_update_node_statistics($nid);
Ideally, there'd be tests for all these. comment_save should produce a
return value, and comment_admin_edit() shouldn't go forward unless
comment_save returns true.
Please excuse my lack of diff/patch-fu. :)
------------------------------------------------------------------------
December 8, 2004 - 09:58 : Junyor
Bah, that function should call comment_save(). All comment changes
should go through comment_save(). It would make life so much easier.
I'll try to come up with an updated patch soon, but probably not before
this weekend.
------------------------------------------------------------------------
December 8, 2004 - 10:28 : Dries
I agree that all comment editing should go through comment_save(), yet
that will require a bit of refactoring. Also, there are two 'edit
comment' forms (one for users, one for administrators) that should be
merged, much like we merged 'node edit' forms. Anyone?
------------------------------------------------------------------------
December 11, 2004 - 17:19 : Junyor
Dries: I see that you made a change to the updating of
node_comment_statistics in CVS HEAD. Do I still need to add an updated
patch? Are there plans for a 4.5.2 that could use this patch?
------------------------------------------------------------------------
December 11, 2004 - 18:14 : ax
i tried upgrading a 4.4 site to 4.5 yesterday and fell into the same
trap as pennywit, kps, junyor, and others: the update for the
node_comment_statistics table only updates forum comments and inserts
num_comments 0 for all other node types. quite cheaty, this.
junyors 4.5 patches (node_comment_stats-2.patch,
node_comment_stats2-2.patch) seem to solve the problem for me. at
least, i see proper "replies" counts in tracker and node views now. i
didn't try approving / publishing / unpublishing comments, though, nor
did i check other issues mentioned in the thread (user names,
postgresql, ...).
i applied the 2 patches to a 4.4 database / 4.5 code both before
running update.php and afterwards. in the first case, there is a small
glitch in that update.php throws an error:
user error: Unknown table 'node_comment_statistics'
DROP TABLE {node_comment_statistics}
because the patch removes "CREATE TABLE {node_comment_statistics}" from
update_105. this could be catched by changing to "DROP TABLE IF EXISTS
{node_comment_statistics}".
this is definitely a critical bug, and these patches should be applied
to 4.5 as soon as possible, regardless of the "all comment editing
should go through comment_save()" issue.
------------------------------------------------------------------------
December 11, 2004 - 20:53 : Jeremy at kerneltrap.org
I've just run into this same bug, trying to upgrade from 4.4 to 4.5.
It's a show stopper for me. :( Ugh, so close.
------------------------------------------------------------------------
December 11, 2004 - 23:29 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats-3.patch (7.25 KB)
Changed patch for updates.inc based on feedback from ax. For 4.5.
------------------------------------------------------------------------
December 11, 2004 - 23:53 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_patch2-3.patch (4.2 KB)
And an update for comment.module with additions to comment_save to
update the node_comment_statistics table. This should cover all edit
cases. For 4.5. Untested.
------------------------------------------------------------------------
December 12, 2004 - 18:40 : Jeremy at kerneltrap.org
I tested the upgrade process on KernelTrap -- worked perfectly nearest I
could tell. I'm also running with your comment.module patch.
------------------------------------------------------------------------
December 15, 2004 - 14:37 : Junyor
What needs to be done for this to be included in core?
------------------------------------------------------------------------
December 15, 2004 - 15:26 : Jeremy at kerneltrap.org
FWIW: In the days following my upgrade, I've found numerous issues with
comment statistics. Specifically, many old postings list "x new of x",
but clicking "x new" shows that in fact none of them were new.
I have not figured out what's different about the nodes for which this
is a problem compared to the nodes for which this isn't a problem. ie,
it doesn't seem to be a problem with a specific node type, etc.
In other words: I retract my earlier comment that the upgrade went
perfectly with this patch applied. Sorry I don't have more specifics
to offer, perhaps I'll have more time at a future date to help dig into
this more. In any case, the patch did improve the process noticeably.
------------------------------------------------------------------------
December 15, 2004 - 15:31 : Junyor
So, none of the comments said they were new or you had read them
previously despite them being marked new?
------------------------------------------------------------------------
December 15, 2004 - 16:13 : Jeremy at kerneltrap.org
> or you had read them previously despite them being marked new?
Right. Comments I had already read when running 4.4 were suddenly
marked as new after the upgrade to 4.5.
------------------------------------------------------------------------
December 15, 2004 - 17:50 : Junyor
That sounds like a problem with history more than
node_comment_statistics. I'm pretty sure this code doesn't do anything
related to marking comments as new or old.
------------------------------------------------------------------------
December 15, 2004 - 21:33 : Dries
If possible provide a single patch against DRUPAL-4-5 and a second patch
against HEAD. Looks like the patches are no longer in sync.
------------------------------------------------------------------------
December 16, 2004 - 10:57 : Junyor
I will do so soon, probably this weekend.
------------------------------------------------------------------------
December 18, 2004 - 01:42 : Jonathan Furness
Ah.... that's why I am struggling with my upgrade from 4.4.2 to
4.5.1..... thank goodness I found this page.
Is there a case for building a script which looks for all the records
in the comments table and updates the node_comment_statistics table...
for those of us who are now working in 4.5.1 with inaccurate records in
node_comment_statistics table?
Looking forward to the fix....
Jonathan
==========
Jonathan Furness
www.jonathansblog.net
------------------------------------------------------------------------
December 18, 2004 - 10:48 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats_4-5.patch (11.45 KB)
Updating patch to current 4.5.1.
Jonathan: Just apply this patch to your installation (from the main
Drupal directory, type 'patch -p0 -u <
../node_comment_stats_4-5.patch') and run update.php.
------------------------------------------------------------------------
December 18, 2004 - 10:59 : Junyor
Dries: The comment.module in HEAD makes use of the cid column in
node_comment_statistics. My patches were removing that column, as it
wasn't being used. Should I leave it alone afterall?
------------------------------------------------------------------------
December 20, 2004 - 07:23 : Dries
The column seems to get populated though (it's not empty). If it the
data is not used, you can remove it.
mysql> SELECT COUNT(cid) FROM node_comment_statistics WHERE cid != 0;
+------------+
| COUNT(cid) |
+------------+
| 3970 |
+------------+
------------------------------------------------------------------------
December 20, 2004 - 09:27 : Anonymous
Well, cid wasn't used until the change you made in revision 1.314 of
comment.module. It was never set correctly or used anywhere in the
code, so I simply removed it.
------------------------------------------------------------------------
December 20, 2004 - 10:40 : Anonymous
Attachment: http://drupal.org/files/issues/node_comment_stats_4-5_0.patch (11.64 KB)
Slightly updated patch for 4.5.1.
------------------------------------------------------------------------
December 20, 2004 - 10:41 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats_head.patch (12.08 KB)
And patch for HEAD (with all CID stuff removed).
------------------------------------------------------------------------
January 4, 2005 - 10:04 : Junyor
Dries: Is there anything else you need here before its commitable?
------------------------------------------------------------------------
January 4, 2005 - 15:31 : ax
i too would much appreciate if this *release* *critical* bug could be
fixed soon. thanks a lot!
------------------------------------------------------------------------
January 4, 2005 - 23:59 : Dries
I look into this patch tomorrow (err, later today).
------------------------------------------------------------------------
January 5, 2005 - 21:37 : Dries
I committed the patch to DRUPAL-4-5. I'm putting the HEAD version on
hold until the revisions patch landed.
Please upgrade your Drupal 4.5 sites to DRUPAL-4-5 in preparation of
Drupal 4.5.2. Thanks.
------------------------------------------------------------------------
January 18, 2005 - 14:47 : rooey
I hate to be a total baboon - but i'm still seeing a problem after
upgrading to 4.5.2 - the "poll" block has no comment options available
(stats or otherwise).
Have i missed something? (or are these patches not included in the
4.5.2 release?)
Thanks!
------------------------------------------------------------------------
January 18, 2005 - 14:54 : Junyor
They are included in 4.5.2. Did you run the update script (it has to be
run despite what the Drupal 4.5.2 announcement says)?
------------------------------------------------------------------------
January 18, 2005 - 15:38 : rooey
Yeah... I did :s
(assuming you're talkin' about update.php)
------------------------------------------------------------------------
January 18, 2005 - 15:40 : Junyor
I'm not entirely sure what this fix has to do with the poll block, as I
don't use that. This fix is to display the correct number of comments
for nodes.
Could you go into some more details about the problem you're having?
------------------------------------------------------------------------
January 18, 2005 - 15:46 : Jeremy at kerneltrap.org
I'm having a similar problem. Note that if you look at the poll in a
node view, the comments show up:
http://kerneltrap.org/node/4536
But if you look at the poll on the front page, it doesn't list any
comments:
http://kerneltrap.org/
I've not taken any time to try and track this down yet.
------------------------------------------------------------------------
January 18, 2005 - 15:46 : rooey
Sure... Since the upgrade from 4.4 to 4.5 the poll block lost all
ability to play with comments.
For example, there used to be the standard "add new comment" links
etc... Now there is not :(
The poll module itself *seems* to be calling the various bits of code
it's supposed to, but alas, nothing is happening.
Take a look at the site if it helps: http://www.lineageone.com
I have jury-rigged the "quote" module to at least get people able to
comment on stuff for now.
------------------------------------------------------------------------
January 18, 2005 - 15:50 : Morbus Iff
Regarding the poll/comment issue, see also:
http://drupal.org/node/14936. Not sure if that bug should be closed and
wrapped into this Issue, or if we should move over to there as a
different issue.
------------------------------------------------------------------------
January 18, 2005 - 16:01 : rooey
Thanks :)
Dang - and they described it so much better than me too!
How do you want to proceed?
------------------------------------------------------------------------
January 18, 2005 - 16:13 : Junyor
Let's take discussions to that bug report. I'll take a quick look in a
couple hours to see if there's anything glaringly wrong.
------------------------------------------------------------------------
February 11, 2005 - 16:50 : Junyor
Attachment: http://drupal.org/files/issues/node_comment_stats_head2.patch (11.92 KB)
Synched patch with HEAD.
------------------------------------------------------------------------
February 11, 2005 - 20:01 : Dries
Committed to HEAD.
------------------------------------------------------------------------
February 27, 2005 - 11:17 : Dries
The upgrade did not work on drupal.org! Marking this both 'active' and
'critical' again. The code does too many queries, and th comment_count
is always 1.
------------------------------------------------------------------------
February 28, 2005 - 17:21 : Junyor
Dries: It looks like you already made the needed changes. Is there
anything else to update?
------------------------------------------------------------------------
March 2, 2005 - 16:17 : ax
this was a *4.5* bug originally. so if we don't want to have a broken
4.5, dries' latest patch [1] should be applied to the 4.5 branch, too.
looking at the cvs log, this hasn't happened yet.
[1]
http://cvs.drupal.org/viewcvs/drupal/drupal/database/updates.inc#rev1.93
------------------------------------------------------------------------
March 16, 2005 - 02:04 : ax
does anyone care to fix this *critical* bug (== apply Dries' HEAD fix)
in 4.5?
maybe setting status to patch will help.
------------------------------------------------------------------------
March 20, 2005 - 17:59 : Junyor
There's a further problem that needs to be addressed, too. If the
comment name has a single quote, the update will throw SQL errors.
More information about the drupal-devel
mailing list