Friday, February 24, 2012

best approach

I've been banging my head for a while now, and it is sore! :-P

I'm a best practice/Microsoft approach type of person and want to make sure I do things correctly.

I have a database, kind of like a forum.

Obviously executing multiple queries in one "batch" (stored proc) would have an impact on the performance.

Now, I would like to give a more detailed/specific error back to the caller (either by aid of error code or whatever) with such situations like...

"EditReply"

Edit reply takes the threadID, replyID and userID.

Before actually commiting the changes, it needs to check:

1) does the user exist in the database? (during the editing of the reply, perhaps the user may have been deleted before running the stored proc, who knows)

2) does the thread exist?

3) does the reply exist?

if the conditions are met, only then will it go ahead and update the database. Now that is 3 queries, and 4 statements overall to make a change to a field/table.

Obviously if one of the commands returns false, in other words if say "does the thread exist" returns 0 (thread doesnt exist) it will return back to the caller an errorcode, which they will handle in their application. Thats all fine but the question is

Am I doing this correctly? (no) - how can I improve this? What do I need to think about?

Of course I would like to give a more detailed error back to the caller (aid of errorcode designed in the application overall) instead of just "no, databases not updated".

In this situation, am I wrongly assuming that the database designers use this type of approach?

Please help, I value your feedback and suggestions. I want to improve and think of the right lines of doing these things.

A lot of the business logic of a database should be encorporated into the table design.

If you insert into a table which is logically linked to another table then there should be foreign keys and check constraints in place to enforce the business logic at table level. This would cover you for user and threads, as the replies table must have a foreign key constraint to the user and threads tables.

if the reply does not exist then nothing is going to get updated anyway, you can use the @.@.rowcount system variable to check how many rows were effected by the last statement.

So with the table constraints enforcing the business logic you should be able to go ahead and make the update regardless of whether it is valid or not. Your application will need to handle SQL errors and report them back to the user and it will need to get that @.@.rowcount system variable to see if the table was actually updated

|||

Many thanks for this

Yes, the tables are all constrainted and relationships are in place and so on.

However I understand I can use the @.@.rowcount to check how many rows where effected by the statement, but if for instance the row was not updated, I would like to know why it was not updated, this could be for example, the user did not exist, in which case I would return an errorcode back to the caller, who handles this particular error code.

I am wondering if this is best design, as the user would like to know what went wrong (such as they got deleted whilst making a change to something, or the thread got deleted whilst they where making a change etc...) therefore in the current stage, there are multiple IF EXISTS statement, and IF EXISTS returns false, they then would get returned back a specific error code from within the application back to the caller.

is this a bad design? how better can they/I make it?

|||

I would also consider looking at another approach wherein the database stored procedures do the data consistency check and return pre-defined error code which the application maintains (via an enum). The application can then check the status code returned by the stored proc and proceed based on that code. Something like this...

enum ReplyOperationStatus {
Success = 0, UserNotFound = 1, ReplyNotFound = 2, ThreadNotFound = 3, UnknownError = 4
};

stored proc : UpdateReply

if not exists (select null from threads where threadId = @.threadId) return 3

if not exists (select null from users where userId = @.userId) return 1

if not exists (select null from thread_replies where threadId = @.threadId AND replyId = @.replyId) return 2

... perform the update ...

return 0

|||

I agree, but flip it around. I will just be generic and use 2000 style errors. Use try...catch if you are using 2005 to catch errors and then you can have a bit more control.

stored proc : UpdateReply

declare @.error int, @.rowcount int

... perform the update ...

set @.error = @.@.error, @.rowcount = @.@.rowcount

if it is an error due to some key violation, it will be raised, so handle it. A good example is having a unique index on a post guid that you generate on the client. This guid would keep them from duplicating a post.

if @.@.rowcount = 0
begin --now see why not
if not exists (select null from threads where threadId = @.threadId) return 3
if not exists (select null from users where userId = @.userId) return 1
if not exists (select null from thread_replies where threadId = @.threadId AND replyId = @.replyId)
return 2
end

return 0

This way you only waste effort to check for things on a failure, which should be infrequent.

|||Wow, thats wonderful. Thanks for the tip Louis.|||Thanks very much, that is pretty much exactly what I already have!! awesome, thanks a bunch :-D|||

Nice expansion on my post with some code there Loius...

I was just wondering though... is it worthwhile having to do those 3 selects in the event of a failure? If the table constraints are set up properly then the SQL Server error message could be just reformatted by the calling application and then returned to the user...

|||Sam - sorry i dont quite follow - do you mean that if there was a SQL general error? if so, then an exception is thrown (assuming we are working with .NET) back to the caller which they handle automatically.|||

Yes a SQL Server generated error

The error text you get when using Query Analyser or Enterprise manager eg:

Server: Msg 547, Level 16, State 1, Line 1
INSERT statement conflicted with COLUMN FOREIGN KEY constraint 'FK_EmployeeSales_Employees'. The conflict occurred in database 'MyDatabase', table 'Employees', column 'EmployeeID'.
The statement has been terminated.

If that is accessible to the callnig application then it could be quickly parsed and returned in a more legible way to the user leaving the data alone for everyone else

|||

I am unsure if you are aware of this, or maybe i've mis interpreted, but that sort of error generated by SQL is thrown back into .NET and will be thrown to the application user so the developer/application has to handle it.

most of the time the error code is also given in the SQLException in .NET so the developer can handle such errors the way they want it.

|||

My thought was that he was doing something like this:

insert into table (columns)
select userId, otherColumns
from user
where user.name = 'fred@.fred.com'

Now, if that user does exist, done. If not, there will be no row inserted. Then you can decide why in the other queries.

No comments:

Post a Comment