issue performing an INSERT statement with mysql_query() PHP

Question

basically I am trying to implement a function in one of my PHP classes that makes an entry into a junction table for a many to many relationship.

This is the method here:

public function setTags($value, $id){
    global $db;
    $tags = $value;
    $query .= "DELETE FROM directorycolumntags 
               WHERE directorycolumn_id = $id; ";
    foreach($tags as $tag){
    $query .= "INSERT INTO directorycolumntags (directorycolumn_id, tag_id)
               VALUES (".$id.",".$tag.");";
    }   
    mysql_query($query);
}

The SQL is produces works fine, as I've echoed it and manually executed it via phpMyAdmin. However, If I leave it as above, the data is never inserted. Does anyone know why this might be happening?

This is the sql it is generating which works fine when I type it manually in:

DELETE FROM directorycolumntags WHERE directorycolumn_id = 178; 
INSERT INTO directorycolumntags (directorycolumn_id, tag_id) VALUES (178,29);
INSERT INTO directorycolumntags (directorycolumn_id, tag_id) VALUES (178,30);
INSERT INTO directorycolumntags (directorycolumn_id, tag_id) VALUES (178,32);

mysql_query does only one statement at a time, it is also deprecated — Musa
– Musa, Commented Nov 6, 2014 at 12:00
nope, you can just concatenate statements and then execute them, its not supported in that API, and stop using that. — Kevin
– Kevin, Commented Nov 6, 2014 at 12:01
multiple queries can't be done using the deprecated mysql extension. You can use mysqli, but you'd be better of deleting, and then using a prepared statement to insert your tags one by one (perhaps in a transaction). Learn the current DB extensions (mysqli or PDO). PS: global is evil — Elias Van Ootegem
– Elias Van Ootegem, Commented Nov 6, 2014 at 12:01
@EliasVanOotegem Out of curiosity, if the original application was built with the mysql extension and I'm just concerned with patching the existing functionality is it safe to use the deprecated extension, until I'm in a position to upgrade the entire app itself? — Javacadabra
– Javacadabra, Commented Nov 6, 2014 at 12:07
@Javacadabra: Using the deprecated extension is not safe. Since PHP 5.5, each mysql_* function call will result in a E_DEPRECATED notice being issued, clogging the logs. The extension itself is scheduled for removal in the future, though no date has been set. You can quite easily update your code to the newer extensions. I prefer PDO, for its clean API and it being, all things considered, easy to learn (check my answer, it contains a full example of how you can do what you're trying to do using PDO) — Elias Van Ootegem
– Elias Van Ootegem, Commented Nov 6, 2014 at 12:13

Elias Van Ootegem · Accepted Answer · 2014-11-06 12:17:02Z

1

The old, unsafe, deprecated mysql_* extension never supported multiple queries. You could, conceivably do this using the mysql replacement extension: mysqli_*, which has the mysqli_multi_query function.
Personally, I'd not use this approach, though. I'd do what most devs would do: use a prepared statement in a transaction to execute each query safely, and commit the results on success, or rollback on failure:

$db = new PDO(
    'mysql:host=127.0.0.1;dbname=db;charset=utf8',
    'user',
    'pass',
    array(
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
    )
);
try
{
    $db->beginTransaction();
    $stmt = $db->prepare('DELETE FROM tbl WHERE field = :id');
    $stmt->execute(array(':id' => $id));
    $stmt = $db->prepare('INSERT INTO tbl (field1, field2) VALUES (:field1, :field2)');
    foreach ($tags as $tag)
    {
        $stmt->execute(
            array(
                ':field1' => $id,
                ':field2' => $tag
            )
        );
        $stmt->closeCursor();//<-- optional for MySQL
    }
    $db->commit();
}
catch (PDOException $e)
{
    $db->rollBack();
    echo 'Something went wrong: ', $e->getMessage();
}

Going slightly off-topic: You really ought to consider using type-hints. From your code, it's clear that $values is expected to be an array. A type-hint can ensure that the value being passed is in fact an array. You should also get rid of that ugly global $db;, and instead pass the connection as an argument, too. That's why I'd strongly suggest you change your function's signature from:

public function setTags($value, $id){

To:

public function setTags(PDO $db, array $value, $id)
{
}

That way, debugging gets a lot easier:

$instance->setTags(123, 123);//in your current code will not fail immediately
$instance->setTags($db, [123], 123);//in my suggestion works but...
$instance->setTags([123], null, '');// fails with a message saying argument 1 instance of PDO expected

edited Nov 6, 2014 at 12:17

answered Nov 6, 2014 at 12:10

Elias Van Ootegem

76.7k10 gold badges123 silver badges160 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Javacadabra Over a year ago

Brilliant Answer, will do, thank you very much for the advice.

Elias Van Ootegem Over a year ago

@Javacadabra: You're welcome, just so you know: if you do decide to refactor the code, switching to either PDO or mysqli, you can post your first attempt here to get some code-review. That way, you'll know if you're going about things the right way

Javacadabra Over a year ago

Oh cool, That's an awesome site, I Wasn't aware of it's existence, will do, much appreciated.

Elias Van Ootegem Over a year ago

@Javacadabra: You're welcome. I can't make promises, but I'll try to review your code if you decide to post it. CR is a great site, with a smaller, but more supportive community than SO, so it's definitely worth a try. It still says "beta", but if you check the meta, you'll see that CR has now officially graduated, and will become a permanent SE site. Oh, and thank you for actually being willing to listen, and learn the new DB extensions. +1 for being reasonable, and willing to put in the work

Serpes · Accepted Answer · 2014-11-06 12:00:53Z

1

http://docs.php.net/mysql_query says:

mysql_query() sends a unique query (multiple queries are not supported) to the currently active database on the server that's associated with the specified link_identifier

If you can use mysqli perhaps this interest you: mysqli.multi-query

Executes one or multiple queries which are concatenated by a semicolon.

answered Nov 6, 2014 at 12:00

Serpes

6724 silver badges14 bronze badges

Comments

Ram Sharma · Accepted Answer · 2014-11-06 12:14:16Z

0

you can not run multiple quires using mysql_query, try to modify your function like this. It would be better if you use mysqli or pdo instead of mysql because soon it will deprecated and it would not work on newer version of php

public function setTags($value, $id){
    global $db;
    $tags = $value;
    mysql_query("DELETE FROM directorycolumntags WHERE directorycolumn_id = $id");
    foreach($tags as $tag){
        mysql_query("INSERT into directorycolumntags (directorycolumn_id, tag_id) VALUES (".$id.",".$tag.")");
    }   
}

edited Nov 6, 2014 at 12:14

answered Nov 6, 2014 at 12:05

Ram Sharma

8,8157 gold badges48 silver badges59 bronze badges

Collectives™ on Stack Overflow

issue performing an INSERT statement with mysql_query() PHP

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related