SQL query - issue with WHERE statement

Question

I have this query, which is made from snippets/ideas around here - I am not an expert in the more advanced SQL yet. The part I want you to focus on is: WHERE ... l.status='active'

SELECT l.*, COUNT(c.id) AS callsNum 
FROM leads AS l 
LEFT JOIN calls AS c ON c.lead = l.id
WHERE l.pool IN ($pools) AND l.status='active' AND l.center='$center'
GROUP BY l.id
ORDER BY callsNum ASC, l.id ASC
LIMIT 0,1

I can't get the error myself, but some people has experienced errors where leads with a status that is not "active" has come up.

Can anyone spot the error? I suppose the code can be better too, any suggestions are welcome - if it ain't clear what the code is supposed to do, feel free to ask.

UPDATE:

Thanks for all your responses. I'm glad you think the query should work. I suppose I need to add some more info.

The system is for a small callcenter. The query is getting the next lead from the selected pools, that the caller should call to. In the same AJAX call to the PHP file the "next lead" gets marked as "processing" to avoid multiple callers getting the samme lead.

As pointed out the problem could technically happen if two callers pressed the "get next lead" button on the exact same time. But they have reported to me, that even if they are just 2 callers calling on the same pools at the same time, they get the same lead quite often. If they are 4 callers, even more often.

I therefore put in some lines of code just after this query, that checks one more time that the lead has status='active' - and if not, it comes up with an error (to prevent multiple callers calling at the same time). This error comes quite often, and I therefore suspect that something is wrong with this query.

It is very important that a lead won't come up multiple times. Any suggestions?

RELEVANT CODE (FULL)

Here is a longer code example as requested. The error in the end comes up quite often with just 2-4 persons using it (on a quite fast server).

// Get next lead
$stmt = $db->prepare("
    SELECT l.*, COUNT(c.id) AS callsNum 
    FROM leads AS l 
    LEFT JOIN calls AS c ON c.lead = l.id
    WHERE l.pool IN ($pools) AND l.status='active' AND l.center='$center'
    GROUP BY l.id
    ORDER BY callsNum ASC, l.id ASC
    LIMIT 0,1");
$stmt->execute();
$res = $stmt->get_result(); 

if($res->num_rows > 0) {

    while($row = $res->fetch_assoc()) {

        // Set as "processing" to avoid simultaneous call from multiple bookers 
        $stmt = $db->prepare("UPDATE leads SET status='processing' WHERE status='active' AND id=? AND center='$center'");
        $stmt->bind_param("i", $row["id"]);
        $stmt->execute();
        $affectedRows = $stmt->affected_rows;
        if($affectedRows != 1) {
            echo 'ERROR. Please reload.';
            die;
        }

    }
}

You are selecting non aggregate columns, which is technically bad, but other than this I don't see any problem. What is your current output? — Tim Biegeleisen
– Tim Biegeleisen, Commented May 12, 2016 at 23:30
@TimBiegeleisen . . . It should be fine in this case, assuming the id is unique in leads. — Gordon Linoff
– Gordon Linoff, Commented May 12, 2016 at 23:33
where looks ok. Can it be that you're getting ` active` or ` active ` columns? — artm
– artm, Commented May 12, 2016 at 23:34
@GordonLinoff I am sure it will work, just pointing out that it should be avoided. — Tim Biegeleisen
– Tim Biegeleisen, Commented May 12, 2016 at 23:34
Does that query fails in execution or it just outputs wrong data? That was not clear! — llouk
– llouk, Commented May 13, 2016 at 0:11

Alex · Accepted Answer · 2016-05-13 03:54:05Z

1

You said:

I can't get the error myself, but some people has experienced errors where leads with a status that is not "active" has come up.

This can be a concurrency problem, where while user A is getting a list of leads (using your query) and reading them, another user (user B) marks one of leads as "inactive" at the same time. So when User A then opens a lead it is marked as "inactive". If User A refreshes a list that lead would no longer be there.

answered May 13, 2016 at 3:54

Alex

5,2223 gold badges24 silver badges44 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Uueerdo · Accepted Answer · 2016-05-13 17:09:26Z

1

I'm not sure if it would have less concurrency issues, but you could try something like this first to 'reserve' the lead:

UPDATE leads 
SET status = CONCAT('Processing ', somesortofcalleridentifier) 
WHERE l.pool IN ($pools) AND l.status='active' AND l.center='$center'
LIMIT 1;

I say like this because your original ordering by a grouping result will probably make it a bit more complicated than what I've shown.

then follow up with:

SELECT l.*, COUNT(c.id) AS callsNum 
FROM leads AS l 
LEFT JOIN calls AS c ON c.lead = l.id
WHERE status = CONCAT('Processing ', somesortofcalleridentifier)
;

answered May 13, 2016 at 17:09

Uueerdo

16k1 gold badge19 silver badges21 bronze badges

6 Comments

Christian Bundgaard Over a year ago

Thanks! If this code is used, how can it get concurrency issues? To me it looks quite good.

Uueerdo Over a year ago

It won't have any that I am aware of, but most of the time I've spent with MySQL was on a project that predated and (as can be guessed) continued to ignore the availability of transactions, etc... So while I don't expect there to be issues, I can't 100% guarantee there won't be.

Uueerdo Over a year ago

@ChristianBundgaard it basically turns the status field into a makeshift mutex; but the more complicated the UPDATE, the more I worry the operation will not be effectively atomic.

Christian Bundgaard Over a year ago

Thanks! It sounds quite promising. I am not familiar with CONCAT - what should "somesortofcalleridentifier" be? Some sort of (quite) unique string for each call, like ex md5(random string + userID)? The query: this way the "callsNum" should be in the UPDATE query, as it is used to determine what lead that should be selected. callsNum is 1st sort-parameter, after that comes id. Is this still possible?

Uueerdo Over a year ago

@ChristianBundgaard I would guess you don't need anything too extravagant for the caller identifier. I would guess you have some sort of unique identifier for each entity that pulls leads in this manner; that should suffice. As far as using the original callNum, you might actually be able to use your original select (but with a larger LIMIT) and just loop through those results attempting an update similar to the one I provided until you get a records affected of 1.

|

Biruk Abebe · Accepted Answer · 2016-05-13 00:35:21Z

0

In the Where part, it says we need table leads which its attributes have some special characteristics to satisfy all three conditions as below:

l.pool should be in set ($pools)
l.status should be active
l.center should be $center

edited May 13, 2016 at 0:35

Biruk Abebe

2,2331 gold badge16 silver badges24 bronze badges

answered May 12, 2016 at 23:43

Golden Hawk

11 bronze badge

1 Comment

Strawberry Over a year ago

Is this an answer?

Collectives™ on Stack Overflow

SQL query - issue with WHERE statement

3 Answers 3

Comments

6 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

6 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related