Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

CDbCommand::execute() logs even caught exceptions #1767

Open
fiesh opened this Issue · 16 comments

6 participants

@fiesh

Given something like this:

try {
        Yii::app()->db->createCommand($command)->execute();
} catch (CDbException $e) {}

CDbCommand::execute() will still log an "error."

This is actually a bug. For example, it is sometimes necessary to INSERT a row and ignore a possible unique violation. With this, the error log will fill up wth nonsense.

@samdark
Owner

What is the reason to ignore validation?

@fiesh

As I said above, you might want to INSERT and row to make sure it is actually there -- even though it might already exist. Or do you mean something else?

@samdark
Owner

I meant why not try selecting it to check if it's there instead of inserting it?

@fiesh

That would lead to race conditions

@samdark
Owner

If you're using MySQL you can use REPLACE or ON DUPLICATE KEY UPDATE.

Overall I understand the issue. Writing to log can be done by default error handler instead of directly in this case. Pretty valid thing to expect.

Will not be able to work on it for 1.1.13 myself. Setting for 1.1.14.

@fiesh

Yes, I'm not using MySQL though, and Postgres doesn't have any such option. Thanks for taking it into account! If you let me know what a reasonable approach would be, I can implement it and make a pull request. One approach would be to add an argument $log to execute() that defaults to TRUE, however thne one has to add it to almost every function in CDbCommand, not sure if that's wise.

@samdark
Owner

Well, there's a way to live with it in PostgreSQL: http://www.postgresql.org/docs/current/static/plpgsql-control-structures.html#PLPGSQL-UPSERT-EXAMPLE

As for Yii logging, I think it's possible to remove all explicit error logging from CDbCommand and move it to CErrorHandler. Not sure about drawbacks currently.

@fiesh

Oh, I didn't think about handling the exceptions in postgresql directly, that's actually a good approach!

The main issue with moving it to the CErrorHandler that I can see is that you somehow need to pass the SQL statement to it.

@samdark
Owner

Can be done by passing it to CDbException.

@samdark
Owner

@qiangxue what do you think? This is definitely interesting approach, I think, that can be used in Yii2. Any drawbacks in general? Any 1.1 specific drawbacks?

@qiangxue
Owner

Yes, I like the idea of moving explicit error logging to the exception handler. In order to do this, the exceptions need to support certain interface so that the exception handler can call them and convert them into strings for logging purpose.

We will definitely consider this for Yii 2. For 1.1, we may consider it in 1.1.14 release probably.

@samdark samdark was assigned
@cebe
Owner

Are we still considering this for 1.1.14? We should get ready for a final release soon.

@samdark
Owner

This one isn't easy to solve and I doubt I can handle it for 1.1.14.

@cebe
Owner

Moved to 1.1.15 then.

@pavel-voronin

Developing over dirty hacks (INSERTS with error supression on specific DB) — very bad practice, be sure. DB returns error, yii have to count with it. Don't want some sort of log records — write LogFilter.

@abhijitpatil-tudip

Hi Everyone,

I am also facing same problem, in which I am handling unique constraint violation to regenerate new values in case of concurrent execution of same code for same set of inputs.

Currently I have no other way to avoid this concurrency issue. So I am am catching exception and reprocessing the inputs to have new unique field values for insertion. This process is recursive until data is not inserted for the given set of inputs.

But with this approach I see the error logs are reported and in my I have configured email logger for errors which is very helpful for me but not in this case.

So if anyone have solution for this please suggest. I am using yii-1.1.14 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.