0

I do a print_r($params); and every value comes through. For some reason it isn't updating the database.

Select statements work with my other PDO statements and I can update items with mysql, but am upgrading those to PDO (such as this statement).

Is there anything wrong with my syntax?

$getData        =       new Data();
$gptest         =       $getData    ->  insert_group_id($item_id, $id, $gp, $groupid, $type);

Data() class includes these:

function insert_group_id($item_id, $id, $gp, $groupid, $type) {

 $params = array(
        'item_id' => $item_id,
        'id' => $id,
        'gp' => $gp,
        'group_id' => $groupid,
        'type' => $type
    );

   $qry = "UPDATE actions_item
            SET gp = :gp ,
                group_id = :group_id ,
                type = :type
            WHERE itemID = :item_id
            AND itemVID = :id
            ";

    return $this->update($qry, $params);        
}

protected function update($sql, $params)
{
    $stmt = $this->dbh->prepare($sql);
    return $stmt->execute($params);
}
  • 2
    http://php.net/manual/en/pdo.error-handling.php --- http://php.net/manual/en/function.error-reporting.php try those if you haven't already. – Funk Forty Niner Aug 29 '16 at 19:03
  • 1
    Are you getting any errors? Make sure you have PDO error signalling enabled as @Fred-ii- shows. – Barmar Aug 29 '16 at 19:29

1 Answers1

2

Your array keys need to have the same format as the placeholders.

$params = array(
        ':item_id' => $item_id,
        ':id' => $id,
        ':gp' => $gp,
        ':group_id' => $groupid,
        ':type' => $type
    );

Based on what I learned instantly after writing this, the colons shouldn't be required. I set up a table with all your same column names and your exact code worked for me without them. So, I'm really not sure why this seemed to solve the problem.

Don't Panic
  • 41,125
  • 10
  • 61
  • 80
  • 1
    No they don't have to. – Funk Forty Niner Aug 29 '16 at 19:04
  • Oh, I thought they did. [example 2 in the docs shows it that way](http://php.net/manual/en/pdostatement.execute.php). Will it work either way? – Don't Panic Aug 29 '16 at 19:06
  • 2
    *"By contrast, the colons are optional when using PDOStatement::bindParam() or PDOStatement::execute(). Both of these work basically identically"* http://stackoverflow.com/a/38925175/ – Funk Forty Niner Aug 29 '16 at 19:06
  • This works! So they have to be there... I didn't think you needed them either. –  Aug 29 '16 at 19:07
  • I guess it's how their system's setup then. No idea why they're "optional" and it works for them. – Funk Forty Niner Aug 29 '16 at 19:08
  • Huh. I didn't realize. Thanks, @Fred-ii-. – Don't Panic Aug 29 '16 at 19:08
  • @Don'tPanic Me neither and that's what is most baffling here. Welcome – Funk Forty Niner Aug 29 '16 at 19:08
  • @Don'tPanic What could be happening here is that (and this is only a guess really), is that since `TYPE` is a MySQL "keyword", it's probably failing on them (silently) because of it and requires that both be set with colons. It's unsure as to what made their code suddenly "work" by them saying it does. – Funk Forty Niner Aug 29 '16 at 19:11
  • @Sol Did you not consult the links I left under your question? And if you did and applied it to your code, did it in fact throw anything? I'm just really baffled at all this, TBH *lol* – Funk Forty Niner Aug 29 '16 at 19:13
  • I couldn't reproduce it. It worked fine for me both with and without the colons. – Don't Panic Aug 29 '16 at 19:22
  • @Don'tPanic Well, I left a comment under Ed's answer http://stackoverflow.com/a/38925175/ and Barmar's. Let's see if he'll get back to us/me. We'll probably never know. – Funk Forty Niner Aug 29 '16 at 19:23
  • Maybe it has to do with the PHP version? I am using 5.5 –  Aug 29 '16 at 19:26
  • @Sol It's very hard to say. As I mentioned above; I left a few comments under another question/answers. We'll see if we can't get to the root of all this. – Funk Forty Niner Aug 29 '16 at 19:26
  • @Sol That's really intriguing. I have never run across an instance where it mattered, and I can't see from the source for PHP itself why it would. Is it definitely on 5.5 (i.e., are you sure that's the version running wherever this code is running)? Also, it likely doesn't matter, but what's your RDBMS? I'm guessing MySQL? – elixenide Aug 29 '16 at 20:08
  • @Sol By the way, as you probably know, 5.5 [reached end of life status last month](http://php.net/eol.php), so it's no longer actively supported. Plus, more recent versions (5.6 and 7.0) are much faster. So, you may want to consider upgrading, regardless of what is causing this particular problem. – elixenide Aug 29 '16 at 20:15
  • @Sol One more question: are you using a framework? More precisely, what is `$this->dbh` in your code sample? Is it a real `PDO` instance, or something else, like a wrapper of some sort? – elixenide Aug 29 '16 at 20:18
  • It is all a PDO statement just put into a couple functions like you see above. MySQL is what I am using. –  Aug 29 '16 at 21:49
  • @EdCottrell it's much simpler. The OP had a typo in their code that accidentally got fixed while adding useless colons. Never base any assumption on the unproved evidence from the unsuspecting user who didn't even manage to get the actual error message before fixing an "error". – Your Common Sense Aug 30 '16 at 04:00
  • @YourCommonSense Are you referring to any specific typo in the code OP actually posted? Or are you assuming that must be what happened? That may well be the case; I'm just trying to determine if you're seeing something I'm not. I don't see any differences between OP's code and the code in this answer, other than the colons. – elixenide Aug 30 '16 at 04:27
  • 1
    @EdCottrell no, I am not seeing anything, my statement is just based on my experience on Stack Overflow. I've seen too many answers that was accepted by opening posters while proposed solution cannot help them in any way or just wrong. So, without any other evidence I would attribute the case to a simple human error. Especially because there is no error message yet. – Your Common Sense Aug 30 '16 at 04:31
  • @YourCommonSense Fair enough; I've certainly had that experience, as well. – elixenide Aug 30 '16 at 04:31
  • 1
    @Sol So, if you change it back, does it stop working again? And did you ever get error reporting turned on and see anything from that? I'm really curious about this, particularly because the answer was based on my misunderstanding of the necessity of the colons. – Don't Panic Aug 30 '16 at 15:17
  • It won't work without the colons, no matter what I have tried. So there is importance to that even if everyone else disagrees. As you have seen the code is pretty straight forward. With my SELECT statement functions though no : colon was needed, so there may be a bug? I think having extra syntax to be more cautious is always better anyways, than lazy programming. –  Aug 30 '16 at 15:44
  • 1
    @Don'tPanic It's not too hard to figure out who gave you a -1 and who gave you at +1. Did you see yours go "up" just now? ;-) – Funk Forty Niner Aug 31 '16 at 16:33