ajoo Posted May 30, 2017 Share Posted May 30, 2017 Hi all, I am doing a multi-table insert transaction like below: I would just like to confirm if this is the right and best way to do this or is there any scope for improvement. I find this a bit tedious (for the poor mysql ) but if this is the normal way of things then it's ok ! $fcon->autocommit(FALSE); $query="INSERT into table (A, B, C, D) VALUES (?, ?, ?, ?)"; $stmt=$fcon->prepare($query); $stmt->bind_param('isss', $A, $B, $C, $D); if($stmt->execute()) { $ID = $stmt->insert_id; $stmt->free_result(); $stmt->close(); } else { $fcon->rollback(); $_SESSION['error'] = " Error inserting record. Try again later."; } $query="INSERT into table1 (ID, F, G) VALUES (?, ?, ?)"; $stmt = $fcon->prepare($query); $stmt->bind_param('iss', $ID, $F, $G); if($stmt->execute()){ $add_id = $stmt->insert_id; $stmt->free_result(); $stmt->close(); }else{ $fcon->rollback(); $_SESSION['error'] = " Record could not be inserted. Try again later"; } $query="INSERT into table2 (ID, H, J) VALUES (?, ?, ?)"; $stmt = $fcon->prepare($query); $stmt->bind_param('iss', $ID, $H, $J); if($stmt->execute()){ $add_id = $stmt->insert_id; $stmt->free_result(); $stmt->close(); $fcon->commit(); return true; }else{ $fcon->rollback(); $_SESSION['error'] = " Record could not be inserted. Try again later"; } Thanks all ! Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted May 30, 2017 Share Posted May 30, 2017 Get rid of the error checks and the manual rollbacks. You should have exceptions enabled by now, and PHP automatically rolls back transactions when an exception occurs. Quote Link to comment Share on other sites More sharing options...
ajoo Posted May 30, 2017 Author Share Posted May 30, 2017 (edited) Hi Guru Jacques !! Yes sir, But how do I then send a sane message to the user ? Like "please try later" etc. . Oh Maybe that should be there just once in the catch block !? right ? Thank you ! Edited May 30, 2017 by ajoo Quote Link to comment Share on other sites More sharing options...
ajoo Posted May 30, 2017 Author Share Posted May 30, 2017 Like this maybe : try{ $fcon->autocommit(FALSE); $query="INSERT into table (A, B, C, D) VALUES (?, ?, ?, ?)"; $stmt=$fcon->prepare($query); $stmt->bind_param('isss', $A, $B, $C, $D); if($stmt->execute()){ $ID = $stmt->insert_id; $stmt->free_result(); $stmt->close(); } $query="INSERT into table1 (ID, F, G) VALUES (?, ?, ?)"; $stmt = $fcon->prepare($query); $stmt->bind_param('iss', $ID, $F, $G); if($stmt->execute()){ $add_id = $stmt->insert_id; $stmt->free_result(); $stmt->close(); } $query="INSERT into table2 (ID, H, J) VALUES (?, ?, ?)"; $stmt = $fcon->prepare($query); $stmt->bind_param('iss', $ID, $H, $J); if($stmt->execute()){ $add_id = $stmt->insert_id; $stmt->free_result(); $stmt->close(); $fcon->commit(); return true; } } catch(exception $e) { $_SESSION['error'] = " Record could not be inserted. Try Later"; $fcon->rollback(); // I wonder if this would be required since I am catching the exception ? } Maybe I should even do away with the if($stmt->execute()) statements too since the exceptions would cover that as well !? Please advise ! Thanks loads ! Quote Link to comment Share on other sites More sharing options...
Solution Jacques1 Posted May 30, 2017 Solution Share Posted May 30, 2017 Get rid of all error checks, try statements etc. Just let PHP and mysqli do their work. If a query fails, mysqli will throw an exception, and if you leave that exception alone, PHP will properly abort the whole transaction. Freeing nonexistent result sets and closing statements which will be closed anyway also seems rather pointless. Manual resource management can make sense for resource-heavy and persistent scripts. But there's no point in cleaning up a simple short-lived script which barely runs for a second. 1 Quote Link to comment Share on other sites More sharing options...
ajoo Posted May 30, 2017 Author Share Posted May 30, 2017 Hello Guru Jacques, I think this would be it then. It still bothers me though, that in case of an exception, 1. The user won't get to see a message informing him that the transaction failed. 2. It would abort the program instead of recovering and allowing the user another chance to retry. Or am I wrong on both counts? I got into this habit of clearing results because it happened a few times that the subsequent queries did not work until I freed the resources with free_result(). I know it was weird since I was not actually storing the results with store_result(). $fcon->autocommit(FALSE); $query="INSERT into table (A, B, C, D) VALUES (?, ?, ?, ?)"; $stmt=$fcon->prepare($query); $stmt->bind_param('isss', $A, $B, $C, $D); $stmt->execute(); $ID = $stmt->insert_id; $query="INSERT into table1 (ID, F, G) VALUES (?, ?, ?)"; $stmt = $fcon->prepare($query); $stmt->bind_param('iss', $ID, $F, $G); $stmt->execute(); $add_id = $stmt->insert_id; $query="INSERT into table2 (ID, H, J) VALUES (?, ?, ?)"; $stmt = $fcon->prepare($query); $stmt->bind_param('iss', $ID, $H, $J); $stmt->execute(); $add_id = $stmt->insert_id; $fcon->commit(); return true; Thank you. Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted May 30, 2017 Share Posted May 30, 2017 1. The user won't get to see a message informing him that the transaction failed. If the server is set up correctly, the user will see the error page and get a 500 status code. I think that's very clear. If you absolutely must have a custom error message, then catch the exception, do what you need to do and finally rethrow the exception, so that it's still properly handled. Simply swallowing it or even continuing the script is a bad idea. 2. It would abort the program instead of recovering and allowing the user another chance to retry. How is the script supposed to recover? If the queries failed, that's it. Yes, the user can try again, but after you've aborted the script. 1 Quote Link to comment Share on other sites More sharing options...
ajoo Posted May 30, 2017 Author Share Posted May 30, 2017 hmm right, That clears it. I am actually re-throwing the exception in my code. But that was for the pictures being stored ( not in the example for brevity ) where I was checking, as shown by you earlier, for a duplicate error name upon insert. So if the error is due to some other reason, it is re-thrown. For the 2nd part, I think i was actually swallowing up the exception, since I was setting up the appropriate message in the $_SESSION['error'] string and then exiting out of the function with a return false; That took the user back to the form and allowed him to try to refill it and submit again. And that you say is incorrect and so it must be. So now when I change that ( to not allow the exception to be swallowed ) I wonder what will happen and if the user will have to get to the home page again and start from there again to fill up the form which I guess must be right. Thanks loads ! Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted May 30, 2017 Share Posted May 30, 2017 None of the queries should ever fail under normal circumstances. All client-side errors should be caught by your validation procedure, so we're talking about an unexpected, major server error here -- like when your database system crashes. It's rather unlikely that this kind of problem can be fixed by simply resubmitting the data. If the user still wants to try, they can use the standard browser navigation: "reload" resends the data, "back" goes back to the form they've filled out. I think that's more than enough. If you still insist on a custom error handling procedure, than you'll have to imitate the behavior of exceptions, which is going to be a lot of work. You have to skip all code which is dependent on the queries, you have to make sure no resources are in an inconsistent state, you have to log or display the error message depending on the PHP configuration, you have to set the right response code. 1 Quote Link to comment Share on other sites More sharing options...
ajoo Posted May 30, 2017 Author Share Posted May 30, 2017 If you still insist on a custom error handling procedure, ... No, I am not insisting on anything Guru Jacques. Just speaking my mind and trying to understand and assimilate all of what you have said. It has cleared most of my doubts. I'll of course go along with your suggestions. Thanks loads !! Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.