New schema for comment_vote that's non-redundant, and more-efficient

(but maybe less-flexible in future, time will tell). Moved the SQL for updating discussion ratings from the .pl to the .pm file, where it belongs (it's not business logic, it's just manipulation of the data).
scc · Sep 6, 2004 · 46f1add · 46f1add
1 parent 664ac89
commit 46f1add
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 58 deletions.
diff --git a/plugins/Rating/Rating.pm b/plugins/Rating/Rating.pm
@@ -68,12 +68,44 @@ sub create_comment_vote {
 
 }
 
+sub getUniqueDiscussionsBetweenCids {
+	my($self, $start_cid, $end_cid) = @_;
+	my $discussions = $self->sqlSelectColArrayref(
+		"DISTINCT comments.sid",
+		"comments, comment_vote",
+		"comments.cid=comment_vote.cid
+		 AND comments.cid BETWEEN $start_cid AND $end_cid");
+	return $discussions;
+}
+
+sub updateDiscussionRatingStats {
+	my($self, $discussions) = @_;
+	return 0 unless $discussions && @$discussions;
+	my $sid_clause = "sid IN (" . join(",", @$discussions) . ")";
+	my $hr = $self->sqlSelectAllHashref(
+		[qw( sid active )],
+		"sid, active, COUNT(*) AS c, AVG(val) AS avgval",
+		"comments, comment_vote",
+		"comments.cid=comment_vote.cid
+		 AND $sid_clause",
+		"GROUP BY sid");
+	my $rows = 0;
+	for my $sid (keys %$hr) {
+		my $sid_hr = $hr->{$sid};
+		my $replace_hr = { discussion => $sid };
+		$replace_hr->{active_votes} =  $sid_hr->{yes}{c} || 0;
+		$replace_hr->{total_votes}  = ($sid_hr->{yes}{c} || 0) + ($sid_hr->{no}{c} || 0);
+		$replace_hr->{avg_rating}   = $sid_hr->{yes}{avgval} || undef;
+		$rows += $self->sqlReplace("discussion_rating", $replace_hr);
+	}
+	return $rows;
+}
+
 sub DESTROY {
 	my($self) = @_;
 	$self->{_dbh}->disconnect if !$ENV{GATEWAY_INTERFACE} && $self->{_dbh};
 }
 
-
 1;
 
 __END__

diff --git a/plugins/Rating/mysql_dump b/plugins/Rating/mysql_dump
@@ -1,5 +1,5 @@
 INSERT IGNORE into hooks (param, class, subroutine) VALUES ("comment_save_success", "Slash::Rating", "create_comment_vote");
-INSERT IGNORE INTO vars (name, value, description) VALUES ('set_disc_rating_last_id', '0', 'last vote_id reconciled');
+INSERT IGNORE INTO vars (name, value, description) VALUES ('set_disc_rating_last_cid', '0', 'Last comment vote cid reconciled');
 
 INSERT INTO string_param (type, code, name) VALUES ("comment_vote", "0", "0 stars");
 INSERT INTO string_param (type, code, name) VALUES ("comment_vote", "1", "1 stars");

diff --git a/plugins/Rating/mysql_schema b/plugins/Rating/mysql_schema
@@ -1,18 +1,9 @@
 CREATE TABLE comment_vote (
-	vote_id mediumint UNSIGNED NOT NULL auto_increment,
-	uid mediumint UNSIGNED NOT NULL,
-	ipid char(32) DEFAULT '' NOT NULL,
-	discussion mediumint UNSIGNED NOT NULL,
 	cid mediumint UNSIGNED NOT NULL,
-	val tinyint DEFAULT '0' NOT NULL,
 	active ENUM("no", "yes") DEFAULT "yes" NOT NULL,
-	ts DATETIME NOT NULL DEFAULT '0000-00-00 00:00:00',
-	PRIMARY KEY vote_id (vote_id),
-	KEY uid_discussion_cid (uid,discussion,cid),
-	KEY ipid_discussion_cid (ipid,discussion,cid),
-	KEY cid (cid),
-	KEY discussion_active_val (discussion, active, val),
-	KEY ts (ts)
+	val tinyint DEFAULT '0' NOT NULL,
+	PRIMARY KEY (cid),
+	INDEX cid_active_val (cid, active, val)
 );
 
 CREATE TABLE discussion_rating (

diff --git a/plugins/Rating/set_disc_ratings.pl b/plugins/Rating/set_disc_ratings.pl
@@ -16,55 +16,21 @@
 $task{$me}{fork} = SLASHD_NOWAIT;
 $task{$me}{code} = sub {
 	my($virtualuser, $constants, $slashdb, $user, $info, $gSkin) = @_;
-	my $last_max = $slashdb->getVar('set_disc_rating_last_id', 'value', 1) || 0;
-	my $this_max = $slashdb->sqlSelect("MAX(vote_id)", "comment_vote") || $last_max;
 
-	my $discussions = $slashdb->sqlSelectColArrayref(
-		"DISTINCT(discussion)",
-		"comment_vote",
-		"vote_id BETWEEN " . ($last_max+1) . " AND $this_max");
+	my $ratings_reader = getObject('Slash::Ratings', { db_type => 'reader' });
+	my $ratings_writer = getObject('Slash::Ratings');
 
-	if (@$discussions) {
-		my $discussion_clause = "discussion IN (" . join(",", @$discussions) . ")";
-		my $summary = $slashdb->sqlSelectAllHashref(
-			"discussion", 
-			"discussion, COUNT(*) AS active_votes, AVG(val) AS rating",
-			"comment_vote",
-			"$discussion_clause AND active = 'yes'",
-			"GROUP BY discussion"
-		);
+	my $last_max = $slashdb->getVar('set_disc_rating_last_cid', 'value', 1) || 0;
+	my $this_min = $last_max + 1; # don't count the last-counted vote twice
+	my $this_max = $slashdb->sqlSelect("MAX(cid)", "comment_vote") || $last_max;
+	return "no new comments" if $this_max < $this_min;
 
-		my $votes = $slashdb->sqlSelectAllHashref(
-			"discussion", 
-			"discussion, COUNT(*) AS votes",
-			"comment_vote",
-			"$discussion_clause",
-			"GROUP BY discussion"
-		);
+	my $discussions  = $ratings_reader->getUniqueDiscussionsBetweenCids($this_min, $this_max);
+	my $num_replaces = $ratings_writer->updateDiscussionRatingStats($discussions);
 
-		foreach my $discussion (@$discussions) {
-			my $avg_rating = $summary->{$discussion}{rating}; # undef/NULL is OK here
-			my $active_votes = $summary->{$discussion}{active_votes} || 0;
-			my $total_votes = $votes->{$discussion}{votes} || 0;
-
-			$slashdb->sqlReplace(
-				"discussion_rating",
-				{
-					discussion => $discussion,
-					total_votes => $total_votes,
-					active_votes => $active_votes,
-					avg_rating => $avg_rating
-
-				}
-			)
-		}
-	}
-
-	my $num = @$discussions;
-	$slashdb->setVar("set_disc_rating_last_id", $this_max);
-
-	slashdLog("$num ratings updated");
+	$slashdb->setVar("set_disc_rating_last_cid", $this_max);
 
+	return "$num_replaces ratings updated";
 };
 
 1;