1

I have a query that I am using to construct a set of data, which supposed to contain exactly the top 3 users by rating per each admin ID. Now because I am clueless how to achieve this using SQL, I am fetching the top users for each admin separately and then pushing them into an array. More over, since calling sth->fetchAll(), and then array_merge(), will lead to having duplicate array keys on the second iteration and onward, and thus will cause a fatal error, I also have an internal iteration(loop) within the first one, which fetches each row from the result set and pushes it into the array where I keep the formatted result. which cause n *3 iterations, which are n * 3 -1 too many, in my humble opinion.

Also, a BTW question that has been bothering me for quite a while now: Is it true that there is no way to bind a parameter or a value to SQL language components such as LIMIT and such with PDO emulated prepared statements disabled?. code:

private function getHotUsers($admins, $count = 3)
    {
        try{
            $conn = DBLink::getInstance();
            $rows = array();
            $sql = "SELECT user_name, user_id, user_group_id FROM users
            WHERE admin_id= :uid  AND status=1 ORDER BY is_hot_user DESC,last_updated DESC LIMIT {$count}";
            $sth = $conn->prepare($sql);
        foreach ($admins as $admin)
        {
            $sth->bindParam(':uid', $admin, PDO::PARAM_INT);
            $sth->execute();
            while($row = $sth->fetch(PDO::FETCH_ASSOC)){
                $rows[] = $row;
            }
        }
        return $rows;   
        }
}

| Field                | Type             | Null | Key | Default | Extra          |
+----------------------+------------------+------+-----+---------+----------------+
| user_id               | int(10) unsigned | NO   | PRI | NULL    | auto_increment |
| admin_id              | int(20)          | NO   |     | NULL    |                |
| user_title            | varchar(450)     | NO   |     | NULL    |                |
| user_desc             | varchar(5000)    | NO   |     | NULL    |                |
| user_data             | longtext         | NO   |     | NULL    |                |
| user_requirements     | varchar(5000)    | YES  |     | NULL    |                |
| user_experience       | varchar(100)     | NO   |     | NULL    |                |
| location_id           | int(11) unsigned | NO   |     | NULL    |                |
| comp_id               | int(11)          | NO   |     | NULL    |                |
| role_id               | int(10) unsigned | NO   |     | NULL    |                |
| user_pass_time        | varchar(100)     | YES  |     | NULL    |                |
| last_updated          | datetime         | NO   |     | NULL    |                |
| is_hot_user           | tinyint(1)       | NO   |     | 0       |                |
| user_internal_id      | int(10)          | YES  |     | NULL    |                |
+----------------------+------------------+------+-----+---------+----------------+

INSERT INTO USERS(admin_id, last_updated, is hot_user) VALUES (1, NOW() - INTERVAL 10 DAY, 1),(1, NOW() - INTERVAL 1 DAY, 0), (1, NOW() - INTERVAL 100 DAY, 1), (1, NOW() - INTERVAL 8 DAY, 0),

(2, NOW() - INTERVAL 1 DAY, 1), (2, NOW() - INTERVAL 100 DAY, 1), (2, NOW() - INTERVAL 5 DAY, 1), (2, NOW(), 0),

(3, NOW(), 0), (3, NOW() - INTERVAL 1 DAY, 0), (3, NOW() - 100 DAY, 1), (3, NOW() - INTERVAL 4 DAY, 0), (3, NOW() - INTERVAL 5 DAY, 0)

Edited as requested by @VolkerK, in bold are the rows that should be selected by the query, the first 3 hot users, that also have the most recent value in their last_updated column, or just the newest users if there are less hot-users tan 3 for this specific admin

Oleg Belousov
  • 9,981
  • 14
  • 72
  • 127
  • 1
    move the `$sth` manipulation outside of the foreach, otherwise you'll lose one purpose of prepared statements... – STT LCU Apr 30 '13 at 12:22
  • 1
    You `prepare` the variable `$sql` yet your SQL string is `$query` – AlexP Apr 30 '13 at 12:23
  • [How can I use prepared statements with LIKE operator?](http://stackoverflow.com/questions/15990857/reference-frequently-asked-questions-about-pdo#15990965) – Your Common Sense Apr 30 '13 at 12:27
  • "Is it true that there is no way to bind a value to LIMIT with PDO emulated prepared statements disabled?" No, you can do so as with any other operator. You just can't use parameter binding for table or column names. – feeela Apr 30 '13 at 12:40
  • Why are all answers getting downvoted on this question without any alternative ideas? One answer even was deleted already. Why? – feeela Apr 30 '13 at 12:45
  • Deleted one was simply wrong – Your Common Sense Apr 30 '13 at 12:47
  • 1
    Can you please post your table structure and some example data? Preferably in the form of `CREATE TABLE ...` and `INSERT INTO ...` sql statements. – VolkerK Apr 30 '13 at 13:01

2 Answers2

0

Nope. it seems that you are already using the right way.
Though you can bind $count as well. Also, although calling $sth->fetchAll(), and then array_merge(), will not lead to having duplicate keys (which is just impossible, mind you), I wouldn't merge all users into single array anyway, but rather group them by their admin

private function getHotUsers($admins, $count = 3)
{
    $conn = DBLink::getInstance();
    $rows = array();
    $sql = "SELECT user_name, user_id, user_group_id FROM topUsers
            WHERE admin_id= :uid  AND status=1 
            ORDER BY is_hot_user DESC,last_updated DESC 
            LIMIT :coint";
    $sth = $conn->prepare($sql);
    foreach ($admins as $admin)
    {
        $sth->bindParam(':uid',   $admin, PDO::PARAM_INT);
        $sth->bindParam(':count', $count, PDO::PARAM_INT);
        $sth->execute();
        $rows[$admin] = $sth->fetchAll();
    }
    return $rows;   
} 

To make some your confusions straight:

  • as a matter of dact, fetchAll does no magic creating an array out of nowhere. It does the same loop, internally. So, there is no overhead.
  • to get top 3 of anything you need to run a separate query anyway. So, it's ok to tun your queries in a loop
  • do not trust your feelings but measure certain numbers. IF this function runs so slow to hinder whole application - go on with optimizations. If not - there are some other things you can spend time for, I believe
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • 1
    "So, there is no overhead" Doesn't perform `fetchAll` the loop on the C level instead of the PHP code level? If it does it is a performance gain in many cases. – feeela Apr 30 '13 at 12:42
-2

I wouldn't execute such a query inside a loop. The first solution is to use … IN (…) to search for a list of ID's (as suggested by Rob).

Or you just set up the query and the parameters in a loop and execute the statement after the loop.

$start = 0;
$limit = 100;
$placeholderArray = $valuesArray = array( );
$i = 0;
foreach( $admins as $adminId )
{
    $placeholderArray[] = sprintf( ' `admin_id` = :admin%d ', $i );
    $valuesArray[sprintf( ':admin%d', $i )] = (int) $adminId;
    $i++;
}

$where = ' (' . implode( ' OR ', $placeholderArray ) . ') ';
$sql = sprintf( 'SELECT user_name, user_id, user_group_id
    FROM topUsers
    WHERE status=1
    %s
    ORDER BY is_hot_user DESC,last_updated DESC
    LIMIT %d,%d;', $where, $start, $limit );

$stmt = $conn->prepare( $sql );
if( $stmt->execute( $valuesArray ) === true )
{
    /* … */
}

The limitation to three entries is achieved by limiting the original input to three entries in the array $admins.

feeela
  • 29,399
  • 7
  • 59
  • 71