-1

simple thing: my code is just not working. Neither INSERT nor SELECT is working in my PDO. Probably I have something wrong, but I'm not a code master, so I need your help.

if (isset($_POST['submit']))
{
try 
    {
        $connection = new PDO('sqlite:../tracker.db');

        $name       = $_POST['name'];
        $unitsize   = $_POST['unitsize'];
        $line       = $_POST['line'];
        $mmr        = $_POST['mmr'];
        $lifespan   = $_POST['lifespan'];
        $connection->exec("INSERT INTO unit (name, unitsize, line, mmr, lifespan) 
        VALUES ('$name', '$unitsize', '$line', '$mmr', '$lifespan')");

        $new_unit = "SELECT unit_id
                     FROM unit
                     ORDER BY unit_id DESC
                     LIMIT 1";
        foreach ($connection->query($new_unit) as $row) {
        $id = $row['unit_id'];
        };

    }
    catch(PDOException $error) 
    {
        echo $error->getMessage();
    }
}

Of course I'm aware that SELECT without records can't work... But my begginer's intuition says, that it could also hava a mistake.

PS: I know, that the code may be a bit messy... sorry for your eyes bleeding :(

EDIT: WHAT I WANT TO ACHIEVE

  1. There is a database tracker.db with existing tables (confirmed by SQLite Browser)
  2. I want to INSERT some data from my form.
  3. After inserting I want to get the last unit_id registered in my DB into variable $id(unit_id is AUTOINCREMENT and PRIMARY KEY)
  4. That's all
  • 1. You should require files based on the submit condition. – Jay Blanchard Sep 11 '17 at 12:47
  • 1
    2. [Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)*** Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Sep 11 '17 at 12:47
  • 3. What do your web server's error logs say? – Jay Blanchard Sep 11 '17 at 12:47
  • Your code makes no sense. You set up the SELECT, but never execute it - clearly it doesn't work. You then exec the INSERT, which probably does in fact work. You then try to read values after that INSERT, but again never run a SELECT to retrieve the data to read, which *again* cannot work. You need to learn to **read** the code you're writing, instead of just pounding away at the keyboard writing random things and hoping they work. – Ken White Sep 11 '17 at 12:52
  • @JayBlanchard nothing, no records –  Sep 11 '17 at 12:59
  • @KenWhite I was sure that `$connection->query($new_unit)` is retrieving data. If I'm wrong, how should I do it? –  Sep 11 '17 at 12:59
  • @KenWhite INSERT is not working. SQLite Browser shows no records –  Sep 11 '17 at 13:02
  • Your code is confusing, e.g. not understandable from the workflow point of view. So, please forget your code for a moment. If you want a proper solution, then reedit your question and explain STEP BY STEP what you wish to achieve. –  Sep 11 '17 at 13:19
  • @aendeerei Reedited code and explanaiton added –  Sep 11 '17 at 13:28
  • Ok. You did a good reedit. Now we can provide you a solide solution. –  Sep 11 '17 at 13:31
  • The code you posted (and that I commented to) did not call `$connection->query($new_unit)`. Look at the revision history. – Ken White Sep 11 '17 at 13:34
  • @KenWhite `$connection->query($new_unit)` was from very beginning. Maybe code was too messy (sorry again for that) and you missed it –  Sep 11 '17 at 13:38

1 Answers1

1

Well, from looking into your code, I could say that the error (at least one) lie in the following parts:

  • Connection creation.
  • SQL statement - the apostrophs (').
  • Uncatched, failure signalizing, returned values of PDO::exec() or/and PDO::query(). You are using both functions in a proper way regarding their definitions, but maybe they returned FALSE on failure. Situation which you didn't handled at all and which is stated in the "Returned Values" parts of their corresponding docus on php.net.

So, because the problem with your code was that you didn't know why it didn't work, e.g. you didn't received any error or sign of it, I thought to show you a complete way to use error reporting + prepared statements + validations + exception handling. Please note that all these four elements are mandatory if you want to code a secure and solid PDO solution. More of it, when you are applying them in a proper manner, you'll always know where a problem (or more) lies. And you'll have a lot more efficiency in code writing, because you'll not loose any more time (sometimes hours!) for finding errors.

Also, how you structure your code is up to you. I presented you here a procedural form, in which you can follow the steps with ease. A better form would be one implemented in an object oriented way.

Recommendations:

  • Always prepare the sql statements (read this) in order to avoid bad intended db injections. In your case, here, this implies that you must use PDO::prepare() + PDOStatement::execute() instead of PDO::exec (read the "Description" from PDO::exec on php.net).
  • PDO is a very powerfull data access abstraction system. But, in order to use it the right way, you need to always read the documentation for each of its function which you are using, and ESPECIALLY the "Return Values" part. This would be a "must", because there are cases when, on failure, a value can be returned in form of a bool FALSE OR an exception can be thrown. These cases must then be properly handled. For example, in case of PDO::prepare():

If the database server cannot successfully prepare the statement, PDO::prepare() returns FALSE or emits PDOException (depending on error handling).

Feel free to ask anything if you have unclarities.

Good luck.

<?php

/*
 * Try to include files using statements 
 * only on the top of the page.
 */
require "../config.php";
require "../common.php";

/*
 * Set error reporting level and display errors on screen.
 * 
 * =============================================================
 * Put these two lines in a file to be included when you need to
 * activate error reporting, e.g the display of potential errors 
 * on screen.
 * =============================================================
 * Use it ONLY ON A DEVELOPMENT SYSTEM, NEVER ON PRODUCTION !!!
 * If you activate it on a live system, then the users will see
 * all the errors of your system. And you don't want this !!!
 * =============================================================
 */
error_reporting(E_ALL);
ini_set('display_errors', 1);

/*
 * ===================================================
 * Two functions used for automatically binding of the
 * input parameters. They are of course not mandatory, 
 * e.g. you can also bind your input parameters one 
 * by one without using these functions. But then
 * you'd have to validate the binding of each input
 * parameter one by one as well.
 *  
 * Put these two functions in a file to be included,
 * if you wish.
 * ===================================================
 */

/**
 * Get the name of an input parameter by its key in the bindings array.
 *  
 * @param int|string $key The key of the input parameter in the bindings array.
 * @return int|string The name of the input parameter.
 */
function getInputParameterName($key) {
    return is_int($key) ? ($key + 1) : (':' . ltrim($key, ':'));
}

/**
 * Get the PDO::PARAM_* constant, e.g the data type of an input parameter, by its value.
 *  
 * @param mixed $value Value of the input parameter.
 * @return int The PDO::PARAM_* constant.
 */
function getInputParameterDataType($value) {
    if (is_int($value)) {
        $dataType = PDO::PARAM_INT;
    } elseif (is_bool($value)) {
        $dataType = PDO::PARAM_BOOL;
    } else {
        $dataType = PDO::PARAM_STR;
    }

    return $dataType;
}

/*
 * ======================
 * Hier begins your code.
 * ======================
 */
try {
    // Read from HTTP POST.
    $name = $_POST['name'];
    $unitsize = $_POST['unitsize'];
    $line = $_POST['line'];
    $mmr = $_POST['mmr'];
    $lifespan = $_POST['lifespan'];

    // Create a PDO instance as db connection to sqlite.
    $connection = new PDO('sqlite:../tracker.db');

    // The sql statement - it will be prepared.
    $sql = 'INSERT INTO unit (
                name,
                unitsize,
                line,
                mmr,
                lifespan
            ) VALUES (
                :name,
                :unitsize,
                :line,
                :mmr,
                :lifespan
            )';

    // The input parameters list for the prepared sql statement.
    $bindings = array(
        ':name' => $name,
        ':unitsize' => $unitsize,
        ':line' => $line,
        ':mmr' => $mmr,
        ':lifespan' => $lifespan,
    );

    // Prepare the sql statement.
    $statement = $connection->prepare($sql);

    // Validate the preparing of the sql statement.
    if (!$statement) {
        throw new UnexpectedValueException('The sql statement could not be prepared!');
    }

    /*
     * Bind the input parameters to the prepared statement 
     * and validate the binding of the input parameters.
     * 
     * =================================================================
     * This part calls the two small functions from the top of the page:
     *  - getInputParameterName()
     *  - getInputParameterDataType()
     * =================================================================
     */
    foreach ($bindings as $key => $value) {
        // Read the name of the input parameter.
        $inputParameterName = getInputParameterName($key);

        // Read the data type of the input parameter.
        $inputParameterDataType = getInputParameterDataType($value);

        // Bind the input parameter to the prepared statement.
        $bound = $statement->bindValue($inputParameterName, $value, $inputParameterDataType);

        // Validate the binding.
        if (!$bound) {
            throw new UnexpectedValueException('An input parameter could not be bound!');
        }
    }

    // Execute the prepared statement.
    $executed = $statement->execute();

    // Validate the prepared statement execution.
    if (!$executed) {
        throw new UnexpectedValueException('The prepared statement could not be executed!');
    }

    /*
     * Get the id of the last inserted row.
     * You don't need to call a SELECT statement for it.
     */
    $lastInsertId = $connection->lastInsertId();

    /*
     * Display results. Use it like this, instead of a simple "echo".
     * In this form you can also print result arrays in an elegant
     * manner (like a fetched records list).
     * 
     * Theoretically, this statement, e.g. the presentation of results 
     * on screen, should happen outside this try-catch block (maybe in
     * a HTML part). That way you achieve a relative "separation of 
     * concerns": separation of the fetching of results from the 
     * presentation of them on screen.
     */
    echo '<pre>' . print_r($lastInsertId, TRUE) . '</pre>';

    // Close the db connecion.
    $connection = NULL;
} catch (PDOException $exc) {
    echo '<pre>' . print_r($exc, TRUE) . '</pre>';
    // echo $exc->getMessage();
    // $logger->log($exc);
    exit();
} catch (Exception $exc) {
    echo '<pre>' . print_r($exc, TRUE) . '</pre>';
    // echo $exc->getMessage();
    // $logger->log($exc);
    exit();
}
?>
  • First of all, thank you. Second: I had to put the part of code in `if(isset($_POST['submit']))` because of notice alert. Your code is working... but I get an exception of not executing `The prepared statement could not be executed!`. I don't need a fancy solution with good security (well, I don't need security at all. Of course I appreciate that you want to teach me good habits), I just need it working. –  Sep 12 '17 at 07:07
  • You are welcome. Yes, i forgot to put that `isset` part. I reedited my answer: I just added in the `catch` blocks the ability to print the whole exception object (do this only on development!). Now you'll see where is the problem. Yes, you needed a working solution. But so you know, all these steps are actually required in a working solution. E.g. they are not fancy stuff. It would be nice to be :-) –  Sep 12 '17 at 07:31
  • The problem is exactly the one I presented in the quote (in my answer), e.g. the statement returned FALSE on failure. Now, let's find the exact problem: –  Sep 12 '17 at 07:54
  • For a start: write values in `$bindings`, instead of variables. Like: `':name' => 'jane', ':unitsize' => 1,...` and try again. –  Sep 12 '17 at 07:59
  • Then take the sql insert statement and run it with the same values directly in the db editor: `INSERT INTO unit (...) VALUES ('jane', 1, ...);` –  Sep 12 '17 at 08:05
  • `attempt to write a readonly database`. Started looking for solution. Changing chmod in terminal seems not working –  Sep 12 '17 at 08:14
  • There you have it. Ok, I'll help you with it by searching a solution, too –  Sep 12 '17 at 08:21
  • `The folder the database resides in must have write permissions, as well as the actual database file.`[from this answer](https://stackoverflow.com/questions/3319112/sqlite-read-only-database) –  Sep 12 '17 at 08:26
  • Actually read all the answers in the link from above. It seems that there may be a lot of reasons for the problem. So, better, you search yourself and try to find a solution by yourself. Otherwise I would just hold you down. If you have any other questions regarding my answer, I'll be glad to help you. Good luck. –  Sep 12 '17 at 08:31