What is the correct way of handling duplicate errors in db

Question

I'm implementing a subscription in a DB. The email must be unique, so I have a UNIQUE index in the database. I have this code in my page init:

$f = $p->add('MVCForm');
$f->setModel('Account',array('name','surname','email'));
$f->elements['Save']->setLabel('Subscribe');

if($f->isSubmitted())
{
    try
    {
         $f->update();

         //More useful code to execute when all is ok :)

    }
    catch(Exception_ValidityCheck $v)
    {
        //Handles validity constraint from the model
        $f->getElement($v->getField())->displayFieldError($v->getMessage());
    }
    catch(SQLException $se)
    {
        //If I'm here there is a problem with the db/query or a duplicate email
    }
}

The only information in SQLException is a formatted HTML message, is this the only way to detect if the error is from a duplicated entry?

Please look at http://fossies.org/unix/www/atk4-4.1.zip:a/atk4/lib/SQLException.php. IMHO I think you may handle them generally. Reporting error is enough already. Or, you can first check if the user email exists or not. — Tommy, Oct 06 '11 at 10:56
I looked at the source before asking here... In the duplicate case is not enough, I need to redirect the user on another page. Maybe SQLException can set the error code properly to reflect the DB error or map it in a more db agnostic way? — The Elter, Oct 06 '11 at 13:00
@romaninsh I'll try to ask more good questions! :P I'll suggest to insert anyway the mysql_errno in the SQLException construction, it could be useful even to detect other fail situations for constraint on the db (like foreign keys contraint). — The Elter, Oct 07 '11 at 10:11

score 1 · Accepted Answer · answered Oct 07 '11 at 00:41

1

Here is one way to do it:

https://github.com/atk4/atk4-web/blob/master/lib/Model/ATK/User.php#L95

Although if you want to perform custom action on duplication, you should move getBy outside of the model, into page's logic.

answered Oct 07 '11 at 00:41

romaninsh

10,606
4
50
70

This is a really nice solution to check constraint in the model. Maybe the exceptions can be made less CPU intensive by collecting backtrace data only on demand, not in the constructor. – The Elter Oct 07 '11 at 10:09
I plan to separate out some exceptions which wouldn't generate the backtrace, but if backtrace is not captured in constructer, there is not much use from it. "Exception_StopInit" is a good example of such an exception which wouldn't need backtrace. – romaninsh Oct 07 '11 at 14:40

score 0 · Answer 2 · answered Oct 07 '11 at 01:00

As @Col suggested, we want to use "insert ignore".

$form->update() relies on Model->update() which then relies on DSQL class for building query. DSQL does support options, but model would generate fresh SQL for you.

Model->dsql() builds a Query for the Model. It can function with several "instances", where each instance has a separate query. I don't particularly like this approach and might add new model class, but it works for now.

Take a look here: https://github.com/atk4/atk4-addons/blob/master/mvc/Model/MVCTable.php#L933

insertRecord() function calls dsql('modify',false) several times to build the query. The simplest thing you could do, probably, is:

function insertRecord($data=array()){
    $this->dsql('modify',false)->option('IGNORE');
    return parent::insertRecord($data);
}

after record is inserted, Agile Toolkit will automatically attempt to load newly added record. It will, however, use the relevant conditions. I think than if record is ignored, you'll get exception raised anyway. If possible, avoid exceptions in your workflow. Exceptions are CPU intensive since they capture backtrace.

The only way might be for you to redefine insertRecord completely. It's not ideal, but it would allow you to do a single query like you want.

I prefer to manually check the condition with loadBy (or getBy) because it takes model conditions and joins into account. For example, you might have soft delete on your table and while MySQL key would not let you enter, Model would and the model-way makes more sense too for business logic.

This is a good "peek under the hood", but it seems a little bit overcomplicating my code... Maybe the exceptions can be made less CPU intensive by collecting backtrace data only on demand, not in the constructor? (https://github.com/atk4/atk4/blob/master/lib/BaseException.php#L37) — The Elter, Oct 07 '11 at 10:16
Just need to define empty collectBasicData here https://github.com/atk4/atk4/blob/master/lib/Exception/InitError.php and similar exceptions. — romaninsh, Oct 07 '11 at 14:42

Your Common Sense · Answer 3 · 2011-10-06T13:04:23.343

-1

Why don't you want to run simple select to check if email is already taken?

Or make it INSERT IGNORE and then check affected_rows

edited Oct 06 '11 at 13:04

answered Oct 06 '11 at 11:11

Your Common Sense

156,878
40
214
345

The purpose of the index is to avoid a select, and in concurrency case is the safest way to handle two concurrent insert of the same value. – The Elter Oct 06 '11 at 13:02
I wouldn't say that index is to avoid a select. Well make it INSERT IGNORE and then check affected_rows, if you don't like select – Your Common Sense Oct 06 '11 at 13:05
Might be a good idea, but this needs to be wrapped up properly into the logic of Agile Toolkit, as you don't just throw naked queries. I'll try to expand on that idea. – romaninsh Oct 07 '11 at 00:44
Ths way can be intersting if is atk can handle with a configuration and if the result can easily "bubble up" – The Elter Oct 07 '11 at 09:59
What I mean by "wrapped up" is that you don't just write queries in Agile Toolkit. You need to call a proper method on proper object and if there is no way, then this should be properly planned on object-oriented level. You have all the power of Object-Oriented programming inside your "Model" and ability to re-define any method helps out in situations like that, when something needs to be done beyond what core developers have coded-in. – romaninsh Oct 07 '11 at 14:45

What is the correct way of handling duplicate errors in db

3 Answers3