2

I've been trying to create a trigger within PHP / PDO. I need to use session variables, and only know how to handle them on PHP, hence this was my start point.

My query is as follows:-

$updateTrigger = "DROP TRIGGER IF EXISTS `trigger_repair_update` ;
CREATE TRIGGER `trigger_repair_update` BEFORE UPDATE ON $tbl_name
FOR EACH ROW
BEGIN
IF (NEW.repaired_by != OLD.repaired_by) THEN
INSERT INTO data_tracking
(`table_name` , `data_id`, `field`, `old_value` , `new_value` , `date_modified`, `username`)
VALUES
('$tbl_name' , NEW.id , 'repaired_by' , OLD.repaired_by , NEW.repaired_by , NOW() , '$user' );
END IF;
IF (NEW.must_have != OLD.must_have) THEN
INSERT INTO data_tracking
(`table_name` , `data_id`, `field`, `old_value` , `new_value` , `date_modified`, `username`)
VALUES
('$tbl_name' , NEW.id , 'must_have' , OLD.must_have , NEW.must_have , NOW() , '$user' );
END IF;
IF (NEW.location != OLD.location) THEN
INSERT INTO data_tracking
(`table_name` , `data_id`, `field`, `old_value` , `new_value` , `date_modified`, `username`)
VALUES
('$tbl_name' , NEW.id , 'location' , OLD.location , NEW.location , NOW() , '$user' );
END IF;
IF (NEW.status != OLD.status) THEN
INSERT INTO data_tracking
(`table_name` , `data_id`, `field`, `old_value` , `new_value` , `date_modified`, `username`)
VALUES
('$tbl_name' , NEW.id , 'status' , OLD.status , NEW.status , NOW() , '$user' );
END IF;
IF (NEW.price != OLD.price) THEN
INSERT INTO data_tracking
(`table_name` , `data_id`, `field`, `old_value` , `new_value` , `date_modified`, `username`)
VALUES
('$tbl_name' , NEW.id , 'price' , OLD.price , NEW.price , NOW() , '$user' );
END IF;
END;";

$sth = $dbLink->prepare($updateTrigger);
$sth->execute();

If I remove the variables and enter into PHPMYADMIN, everything works fine, so my conclusion is that I have not allocated the variables properly. Should I be binding parameters here?

I've looked in many places for this:

I still seem to be missing something though.

EDIT:- Thanks to Michael Berkowski, I have added the error code and get the following message:-

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'CREATE TRIGGER trigger_repair_update BEFORE UPDATE ON repair FOR EACH ROW BEGI' at line 2' in /Applications/MAMP/htdocs/repair_list.php:58 Stack trace: #0 /Applications/MAMP/htdocs/repair_list.php(58): PDO->prepare('DROP TRIGGER IF...') #1 {main} thrown in /Applications/MAMP/htdocs/repair_list.php on line 58

Line 58 is where the $sth = $dbLink->prepare($updateTrigger); sits.

Community
  • 1
  • 1
Jason
  • 159
  • 2
  • 4
  • 12
  • 1
    Have you checked for errors? PDO errors silently by default. `$dbLink->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);` to make it throw a meaningful exception. – Michael Berkowski Mar 12 '15 at 16:38
  • @MichaelBerkowski - yes I have. I don't get an error, the screen just goes blank. I thought this odd, however I don't know how to force an error to be reported. At least, I've set the attribute as you suggested above. Should I be adding in anything else to capitalise on the attribute? – Jason Mar 12 '15 at 16:43
  • Can you echo the statement with the variables expanded by PHP, and then paste that into phpMyAdmin? Also, have you properly escaped the variables? If they have an apostrophe in, it will break, and it will likely introduce a SQL injection vulnerability. – halfer Mar 12 '15 at 16:48
  • 2
    Enable display_errors in PHP. At the top of your script, `error_reporting(E_ALL); ini_set('display_errors', 1);` However, if there is a parse error in the code, that won't be printed on screen. You would have to look in the error log. – Michael Berkowski Mar 12 '15 at 16:49
  • @halfer - yes, when I replace the variables with real values and enter in phpMyAdmin the Trigger is set correctly, does what it should and is visible with 'Show Triggers' in phpMyAdmin. There are no apostrophes in the variables, most are single words. I'm using them to track the user in session variables and then write who changed what using the trigger. That's the plan anyway :) – Jason Mar 12 '15 at 16:54
  • @MichaelBerkowski - good call. I've added that and now get the message as per edit to my question above. – Jason Mar 12 '15 at 16:56
  • 1
    Perhaps the user you are using in this script doesn't have database permissions to create triggers? After you do the `execute()` you can ask the driver for any errors that may have occurred. This method returns a boolean success flag, too. – halfer Mar 12 '15 at 16:56
  • 1
    You cant run multiple statements with PDO prepare – Mihai Mar 12 '15 at 17:03
  • @halfer - The user is only for the PHP code and isn't really password protected. The database is accessed using $dbLink = new PDO ('mysql:host=localhost;dbname=overtime;charset=utf8', 'username', 'password'); If I understand correctly, this is where the DB user is set unless the $user variable conflicts with this in MySQL (a reserved variable or similar?) – Jason Mar 12 '15 at 17:03
  • 2
    D'oh - it's obvious now. PDO does not support multiple queries in the prepared statement. You are executing a `DROP` followed by a `CREATE`. Separate the actions. Call the `DROP` with `$dbLink->exec("DROP...")` first, then prepare the `CREATE`. Technically since you have no bound parameters, you could use `exec()` for the `CREATE` as well. – Michael Berkowski Mar 12 '15 at 17:03
  • @Mihai - and Michael - Oh. I had hoped I could. The question here http://stackoverflow.com/questions/11247045/why-cant-i-create-a-trigger-with-pdo seemed to indicate that I could. Would I be able to do it in a function and then calling the function? – Jason Mar 12 '15 at 17:06
  • This all assumes you have carefully validated the contents of `$user` and `$tbl_name` and any other variable I'm not seeing in there. Where `$tbl_name` is used as a `VALUES()` value, it really should be bound with a placeholder for `prepare()`. And _yes_ you can wrap the two actions in a function and call that. – Michael Berkowski Mar 12 '15 at 17:07
  • I`m not sure how you can run mutiple statements in one go in PDO,one solution si to change it to mysqli with mysqli_multi_query,not sure why PDO hasnt got something similar – Mihai Mar 12 '15 at 17:07
  • @MichaelBerkowski - (and Mihai) I'm with you. Good thought. I'm struggling a bit with PDO, although don't want to rely on old faithful mysqli_query :) – Jason Mar 12 '15 at 17:08
  • @MichaelBerkowski the error I'm getting suggests that at least the $tbl_name variable contents are being passed OK. I'm going to try using a function and will also try binding the variables as you suggest. It may take me a while however I will report back. – Jason Mar 12 '15 at 17:12
  • Whilst it looks like someone has spotted other problems, nevertheless it does not matter that the user `username` has a trivial password (or no password at all). The database `overtime` may be accessed by one or more users, who will have specific `GRANT` permissions on the database, which may or may not include the ability to create triggers. This may still be the problem, depending on the error you find by echoing it from the driver. – halfer Mar 12 '15 at 17:13
  • @halfer - yeah, I took the real username and password out for the sake of asking here :) I'll have a think about DB permissions and user access as it's probably a future step. Thanks though, it's all useful in the thought melting pot :) – Jason Mar 12 '15 at 17:16
  • Yes, security is a future step. But it may also be a problem **right now**, that's my point - if your user doesn't have permissions to create triggers, it won't be able to (I guessed that `username` was not the real name - not the point I am addressing at all!). – halfer Mar 12 '15 at 17:22
  • @halfer - I'm not sure I understand. Also, I recognise that my code will almost certainly not be clean. I'm working in a completely closed environment with all the PHP and HTML sitting on a server sitting within a LAN. There are no usernames or passwords and the DB is accessed as per my above comment. There are several local machines used to access the DB although there are no passwords, just usernames held in the $_Session variable on each machine. The username / password combination in the DB connection should have root priviliges, so wouldn't all local machines access as that same user? – Jason Mar 12 '15 at 17:28
  • I've just changed to $dbLink->exec(DROP... and $dbLink->exec(CREATE as per Michael's suggestion and everything works as I wanted, however you have all raised some great points for me to think about, so I will have a longer think and sanitise the code more thoroughly. Thanks to all. – Jason Mar 12 '15 at 17:44
  • _"I'm not sure I understand... There are no usernames or passwords... The username / password combination in the connection should have root privileges"_. I don't know if you're aware of this, so I'll state it for the purposes of being helpful: there's _always_ usernames to access a database server, and it _always_ has a set of privileges. You've now indicated that this user has root permissions, so my wondering whether you were hitting permissions problems can, finally, be ruled out. Got there in the end `:-)` - pleased you fixed it! – halfer Mar 12 '15 at 19:20
  • 1
    @halfer of course :-) I sort of knew but hadn't worked it through as articulately in my mind. Thank you all for all of your help :-) – Jason Mar 13 '15 at 07:11

1 Answers1

1

(Adding answer for the sake of completeness, from Michael's comment).

PDO does not support multiple queries in the prepared statement. You are executing a DROP followed by a CREATE. Separate the actions. Call the DROP with $dbLink->exec("DROP...") first, then prepare the CREATE. Technically since you have no bound parameters, you could use exec() for the CREATE as well.

halfer
  • 19,824
  • 17
  • 99
  • 186