SELECT failing to find entry

I’ve used SELECT many times and I can’t figure out why this one function is failing to find data that is there. Please note in the code examples that the original intent of the functions is temporarily put aside for the purposes of debugging this problem.

The database table is called “monsterlist”. The table contains many columns, the first two being “ID” and “Name”.

The function I have created is short and simple:

    function MonsterGetIDfromName($name)
    {
      global $db;
      $dq = $db->prepare("SELECT ID FROM monsterlist WHERE Name =".$name);
      $dq->execute();
      $ID=$dq->fetch();
      if($ID)
      {
        return "found something";
      }
      $ID=$ID["ID"];
      $dq->closeCursor();
      if($ID)
      {
        return $ID;
      }
      else
      {
        return $name;
      }
    }

The test code in the page is:
$encounter_info = MonsterGetIDfromName(“Goblin”);
and somewhat later:
echo (“window.alert(’”.$encounter_info."’);\n");

Running the page results in a window.alert with “Goblin” shown, which indicates that function failed to set $ID equal to anything.

I have verified via phpmyadmin that for ID=156, Name=Goblin.

EDIT: Sorry for the sloppy-looking code, it says entering 4 spaces before a line makes it auto-format as code but that doesn’t seem to be working.

You ALWAYS need to have error handling for statements that can fail due to errors. The easiest way of adding error handling for database statements is to use exceptions and in most cases let php catch and handle the exception where it will use its error related settings to control what happens with the actual error information (database errors will get displayed/logged the same as php errors.)

For the PDO extension, you can set the error mode to exceptions when/after you make the connection.

Once you do this, you will be getting an error about an nonexistent column named - Goblin. The reason for this error is because the piece of data is a string and it needs to be treated as a string so that it won’t be seen as a column name. If you were correctly using a prepared query, you wouldn’t be getting this error because you wouldn’t be putting the data value directly into the sql query statement.

1 Like

I’ve always been really confused by error handling code, so I’ve kinda skipped it and hoped for the best in the past. I’ll try and take a look at it again, but it went way over my head last time I looked at it.

When you mention “putting the value directly into the sql query statement” are you talking about doing it like “Name = :name” instead, and then using the Bind_Value or Bind_Param? I was told to use prepared statements by a friend to prevent SQL injection, and even though it’s not possible for this function (the paremeter this function takes is taken from a table itself and there isn’t anything that can write to that table), I got used to using prepared statements all the time. But since it wasn’t a vulnerable function I thought I could just use the variable directly. Okay, so that’s something new for me know. Thank you.

Prepared statements separate the data values from the sql syntax. They prevent any sql special characters in the data from breaking the sql query syntax, which is how sql injection is accomplished. If any of these names could contain even a single-quote, it would break the sql syntax and produce an error. Use a prepared query for anything that could ever contain any sql special characters.

Good database design would store the id values (auto-increment primary index) in related tables, not the name value. The only time you would deal with a name value as an input to a SELECT query would be if you are searching, in which case the name would be coming from external data and could contain anything.

Using exceptions for errors simplifies all your error handling logic (you don’t have to have any in most cases.) Most database related errors (connection, query, prepare, and execute) are fatal and nonrecoverable for a database dependent page. Your system should log the actual information about the error and the page should just produce a http 500 error. Telling legitimate visitors anything more then that the page doesn’t work is pointless, and telling a hacker that something they did caused a specific problem will just get them to do more of the same. By letting php catch and handle the exception, all of this happens ‘automatically’. The exception to this is when inserting/updating user submitted data and a duplicate or out of range error occurs. This is a recoverable error. In this case, your code would catch the exception, detect if the error number is for something it can handle, then setup a user message telling exactly what was wrong with the submitted data.

If you use implicate binding, by supplying an array of data to the execute() call, you don’t have to waste time with bind_ statements. Keep It Simple.

Yes, in general I use the numerical ID to search for a specific row in the table, however there is one exception and I created this function to handle that. There is a column that contains a string of Names (matching entries in the Name column) separated by spaces. This is so that when data for a row is called, I can immediately relate it to other rows. I could use IDs separated by spaces instead, but that would make it a lot more cryptic to me when viewing it. I would not be surprised if there is a better way to do that. Almost every day I learn something about programming that I never knew before.

$pdo = new PDO('mysql:host=' . DATABASE_HOST . ';dbname=world;charset=utf8', DATABASE_USERNAME, DATABASE_PASSWORD, $db_options);

$query = 'SELECT Name, CountryCode, District, Population FROM city WHERE CountryCode=:CountryCode ORDER BY District'; // Set the Search Query:

$stmt = $pdo->prepare($query); // Prepare the query:

$result = $stmt->execute([':CountryCode' => filter_input(INPUT_POST, countryCode)]); // Execute the query with the supplied user's parameter(s):

You can do something like the above and simply do the following:

          if ($result) {
                    while ($record = $stmt->fetch()) {
                        echo "<tr>";
                        echo '<td>' . $record->Name . "</td>";
                        echo '<td>' . $record->CountryCode . "</td>";
                        echo '<td>' . $record->District . "</td>";
                        echo '<td>' . $record->Population . "</td>";
                        echo "</tr>";
                    }
                } else {
                       echo "Country Code not found!<br>";
                }

I wouldn’t worry about using exceptions as long as you have error reporting when testing the code.

This smells of a bad database design.

1 Like

Your immediate in that statement isn’t as fast as you might think. Because this design cannot use an index, this is the slowest possible way of finding data and your code to manage the list of values stored in a single column is more complicated. You should store the list of character ids, one per row, in a foreign table, related back to whoever they belong to through the user’s id. This will result in the fastest queries, the least amount of data storage, and the simplest code.

What matters is how quickly the database can relate data to other rows, as that is the normal way it will be accessed on a web site.

Very likely. I admit I know very little of what I’m doing, I’m a learn-as-I-go type when it comes to programming and most of what I learn is by trial and error. What I was aiming for was to be able to store an array into a single data field for each row in the table. There’s no way to do that, though, right? So I simulated an array by using a string of tokens separated by spaces, which the code can explode() later when I need to use it.

What’s the general practice for what I’m trying to do in that case? If you have a table and your function determines that the data in a row is going to be used, it must also get the data from any rows required by the first row (and those could also have requirements of their own), how should these requirements be indicated in the table?

Point taken on this, but in the case of this project I’m not worried at all about speed. This is meant to be something where you click a button and then wait a bit for it to process the final results. I’ve had it do a test generating hierarchies for every row in the table, one after the other, and it’s within acceptable time constraints.

Thank you both for the help.

Lets start with the DB. Post an SQL dump of your DB along with a few sample records.

The error is that $name needs to be quoted:
$dq = $db->prepare(“SELECT ID FROM monsterlist WHERE Name = '$name');

I am not addressing any of the other questions by others, but this is definitely an issue.

That would already been done if Prepared Statements are used properly:

$dq = $db->prepare("SELECT ID FROM monsterlist WHERE Name = ?");
$dq->execute([$name]);

Otherwise this should lead to a hard error if PDO is configured the right way for a testing system:

$pdo = new PDO('mysql:host=localhost;dbname=someTable', 'username', 'password', [
  PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
]);
Sponsor our Newsletter | Privacy Policy | Terms of Service