MyBB Community Forums

Full Version: Unused $referrals in member.php
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Going through some debugging in mybb pages and I found this.

member.php
	$query = $db->simple_select("users", "COUNT(uid) AS referrals", "referrer='{$memprofile['uid']}'");
	$referrals = $db->fetch_field($query, "referrals");

I have never seen $referrals shown anywhere in profiles and the variable $referrals has no other references in the file.

Am I missing something or was this intended to be used and never was. It should be removed if it's not used.

This query scans the entire users table and has the longest query time for that page on my site. It doesn't use a key/index either. A complete waste.
Looks to me like it counts the number of referrals that a user has (I'm positive you've figured that out). Maybe they were going to have it displayed somewhere and forgot, or maybe they were going to do something with it, decided against it, were going to remove it... and forgot. Of course, I don't know for sure so don't listen to me.

If removing it does not affect the script, I'm all for dumping it even if development is focusing on 1.6. Cleaner code is always a good thing.
(2010-01-09, 02:07 AM)Firestryke31 Wrote: [ -> ]If removing it does not affect the script, I'm all for dumping it even if development is focusing on 1.6. Cleaner code is always a good thing.

And it would need to be removed in the 1.6 trunk as well, yes?
I think it was going to be used but never was. It should be removed. This query on 120,000 rows took .5 seconds which is 10 times longer than all the other queries.

Of course I'd like the query to be used but it's more likely to be removed in 1.4x branch and later used in 1.6x branch. But I think it's weird it made it this far in the 1.4x branch and was never used.

I just found another oddball query in memberlist.php.

$query = $db->simple_select("users u", "COUNT(*) AS users", "{$search_query}");
$num_users = $db->fetch_field($query, "users");

That should be optimized to use $db->num_rows at least but I think it could also be a better query. The select could just grab uids imho.

$query = $db->simple_select("user", "uid", "{$search_query}");
$num_users = $db->num_rows($query);

And here is another one from member.php.

$query = $db->simple_select("users", "*", "regip='".$db->escape_string($session->ipaddress)."' AND regdate > '$datecut'");
$regcount = $db->num_rows($query);

Basically it grabs all data * when it could just grab the uid, regdate and ipaddress imho. I personally dislike seeing a grab of all data when it's not neccessary. I have to assume this will take up extra resources which as mybb sites grow become precious.
Quote:This query on 120,000 rows took .5 seconds which is 10 times longer than all the other queries.

Really?
Thats big enough
I have had 147,000 users join but I delete unactivated accounts. Surpisingly out of my 1.8gb database the user table is only 50mb.
(2010-01-09, 02:21 AM)labrocca Wrote: [ -> ]I think it was going to be used but never was. It should be removed. This query on 120,000 rows took .5 seconds which is 10 times longer than all the other queries.

Of course I'd like the query to be used but it's more likely to be removed in 1.4x branch and later used in 1.6x branch. But I think it's weird it made it this far in the 1.4x branch and was never used.

I just found another oddball query in memberlist.php.

$query = $db->simple_select("users u", "COUNT(*) AS users", "{$search_query}");
$num_users = $db->fetch_field($query, "users");

That should be optimized to use $db->num_rows at least but I think it could also be a better query. The select could just grab uids imho.

$query = $db->simple_select("user", "uid", "{$search_query}");
$num_users = $db->num_rows($query);

And here is another one from member.php.

$query = $db->simple_select("users", "*", "regip='".$db->escape_string($session->ipaddress)."' AND regdate > '$datecut'");
$regcount = $db->num_rows($query);

Basically it grabs all data * when it could just grab the uid, regdate and ipaddress imho. I personally dislike seeing a grab of all data when it's not neccessary. I have to assume this will take up extra resources which as mybb sites grow become precious.

If you look further down, MyBB probably iterates through those queries. Thus any changes you make would break that functionality. I don't have enough time to check it right now though, so I can't be sure.

Either way, if there is anything I have learned from coding, changing anything even if it looks innocent, will likely cause more regressions in the future.

Second of all, using db->num_rows is actually slower than doing a count. I've already explained this to you in the bug report you recently submitted.

As for your first post in the thread, I'll have to take another look to ensure it's not being used, but if it isn't I'll remove it for 1.6/1.4.

Ryan
Your explanation for why the change isn't optimal is understood. $num_users only appears to be used to create the multipage a few rows down. It only requires a number and the $query is not used anywhere else.

Also the original $referrals that I mentioned in this thread is still unused. I am reporting it now.