Why is this query not working?


#1

I have many similar queries that are working, but this one is not. I have checked for syntax errors well over a dozen times. I just can’t figure out what is wrong with this code (note: I added the declaration of $setSelection for the purposes of this post):

$setSelection = '47';
$result2 = 'SELECT * FROM tableMinis WHERE mini_set_id = :selection;';
$pdo->prepare($result2)->execute(['selection'=>$setSelection]); 
while($row2 = $result2->fetch()) {
     echo $row2["mini_name"] . '<br>';
}

To test it, I simplified the query and it works fine this way:

$result2 = $pdo->query("SELECT * FROM tableMinis WHERE mini_set_id = '47';");
while($row2 = $result2->fetch()) {
     echo $row2["mini_name"] . '<br>';
}

Here is the error message:
Fatal error: Call to a member function fetch() on a non-object in /home/ubuntu/workspace/minisgallery v2/Live Files/controls/blank.php on line 67 Call Stack: 0.0011 239848

(Note: line 67 is the while statement)

Here are the options I am running on my database connection:

    $options = [
        PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
        PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
        PDO::ATTR_EMULATE_PREPARES   => false,
    ];

#2

I tried a different way of coding this, and it works:

$result2 = $pdo->prepare('SELECT * FROM tableMinis WHERE mini_set_id = :selection');
$result2->execute(['selection' => $setSelection]);

Could someone please tell me why though, that the other way is not working? I have 50+ other queries formatted the other way, so if there is a risk that they might fail as well, then I better start changing them all over now.


#3

You are mixing up variable usage and not looking at the code after it produced the error. $result2 in the bad code is the sql query statement, which is a string, then since you didn’t assign the prepared query PDOStatement object to a variable, you ended up using the variable holding sql query string to try to call the fetch() method. I recommend picking a variable naming scheme and using it throughout your code. Also, if you don’t understand method chaining, don’t use it.

Use $sql for the sql query statement (there are debugging and programming advantages to producing the sql query statement in a variable.) Use $stmt for the PDOStatement object. Use an appropriately named variable, that indicates the meaning of the data, for the fetched data.

Also, you should separate the different concerns in your code. The database specific code, that knows how to query for and retrieve data is a different concern from the presentation code, that knows how to produce the output from the data. The way to do this is to simply fetch the data from any SELECT query into an appropriately named variable, then use that variable in the html document where you are going to display the output. There are three main reasons to do this - 1) it forces you to define what data you are using, 2) allows the design and testing of the two different concerns to be done separately, and 3) if you ever need to change the data source, for example, getting the data from an API instead of a database, all you need to change is the code that’s responsible for getting the data, you don’t have to touch the code in the html document.

Next, if you find yourself creating variables that end in a sequence of numbers, it is a sign you are doing things the hard way. You should finish with one query before going onto the next. Separating the different concerns will help avoid the need to use variables ending in numbers.


#4

Thank you phdr for your detailed explanation, however I am having difficulty understand some of it. For learning purposes I thought I would break it down as follows:

I don’t understand why prepare($result2) does not then bring in the assigned result from this string variable. It works fine the 50+ other times I am using it – why not this time?

(Sorry, you really lost me on the rest of line). This code format wasn’t even my format – it was the format recommended by an advanced coder – why would this format not work if its so highly recommended?
I now have to go through all my pages and change 50 instances of this code, because 1 instance used the same way as the rest won’t work – very frustrating.

I have a primary query whose elements are referenced throughout a while loop, and then I have several secondary queries that are only referenced at the time the query is made at various stages throughout the loop. Because of this, I do need two separate variables for these, however for the secondaries I have been incrementing $resultX and $rowX, when I realize now the incrementation does not need to be done.

Not really sure what this really means. (Sorry, but a damn stroke really messed up my cognitive processing). I’m guessing that “prepared” and “execute” are methods? Or are they functions? I really wouldn’t know when I am method chaining or not, since I don’t know the different between the two. I’m guessing if I am ever using a statement that includes more than 1 use of “=>” then I am chaining and apparently that’s bad. I don’t understand why breaking them apart into 2 different lines would be any different than chaining them – or is it just a flaw in the way the code was developed?

I wasn’t aware that $sql or $stmt did anything special – I thought they were just standard go-to variable names?

So for any non prepared statement queries, I should use $sql for the query statement, and for prepared statement queries I should use $stmt?

Lets use the working code as an example:

$result2 = $pdo->prepare('SELECT * FROM tableMinis WHERE mini_set_id = :selection');
$result2->execute(['selection' => $setSelection]);

Based on what you said above, this value should be $stmt instead of $result2?

However, you have also indicated that I should use more descriptive variables, such as $getMiniInfo instead of $result2?

Which is it?

Or do you mean, after I have obtained the $stmt result, then set it to a descriptive variable ($getMiniInfo = $stmt)? Although, after checking phpdelusions, it looks like this should be $getMiniInfo = $stmt->fetch(); ?
Would this then mean that I don’t need to use the fetch reference in my while loop?

Would: while($row = $result2->fetch()) {
Become: while($row = $getMiniInfo) {
?

As mentioned above in 2., I have a main query and several secondary queries which compare data against the results in the main query. How would this apply to number row results, such as in this example:

<option value="<?php echo $row2["make_id"] ?>"<?php if ($row2["make_id"] == $row1["line_make_id"]) { ?> selected<?php } ?>><?php echo $row2["make_name"] ?></option>

Thanks again for your time in helping me try to understand this.


#5
$result2 = 'SELECT * FROM tableMinis WHERE mini_set_id = :selection;';
$pdo->prepare($result2)->execute(['selection'=>$setSelection]); 
while($row2 = $result2->fetch()) {

Here you set $result2 to be a string with a sql query.

You then use it in the PDO->prepare method. Btw: a method is just a function that is inside a class. In this case prepare is a method (function) inside the PDO class.

Then lastlty you are trying to call a fetch method (function) of the string in $result2. A string in PHP does not have any methods. You have basically just fooled yourself because of bad variable naming to think that you had a DB result available.

This code should be rewritten into something like this:

$setSelection = '47';
$sql = 'SELECT * FROM tableMinis WHERE mini_set_id = :selection;';
$stmt = $pdo->prepare($sql)
$stmt->execute(['selection'=>$setSelection]); 
while($mini = $result->fetch()) {
     echo $mini["mini_name"] . '<br>';
}

I would advise you to reconsider the naming scheme of your db though, there is no real reason to prefix tables with table, or prefix every column with the table name.

I also changed it to use unnamed placeholders, I just prefer that over named placeholders. Guess it’s nice for you to be able to consider the alternatives.

I also prefer to just fetch all, I feel it gives me an easier time naming things appropriately.

Also note how I’ve split the logic and the view/template. If you name things properly then you may find your code gets a lot cleaner if you have your logic at the top of the file and simply output data at the bottom. This will also allow you to break out of PHP and output HTML instead. One advantage being that you avoid nesting quotes which you may often messes up your code, and your code editor has an easier time coloring/autocompleting/etc. Note that the html output below may be plain gibberish as I know nothing about your data structure.

$setSelection = '47';
$sql = 'SELECT * FROM minis WHERE set_id = ?';
$stmt = $pdo->prepare($sql)
$stmt->execute([$setSelection]); 
$minis = $stmt->fetchAll();

// more PHP logic here
// ie fetching more data from the database, $makes, etc

// only output under this line
?>

<?php foreach($minis as $mini): ?>
    <h2><?= $mini['name'] (<?= $mini['id'] ?>) ?></h2>
    <p><?= $mini['description'] ?></p>
<?php endforeach; ?>

#6
  1. This is method chaining

$pdo->prepare($result2)->execute(['selection'=>$setSelection]);

In this code you call the prepare method in the object in $pdo. But what context is the execute method called in…?


#7

Question #1: Do you forsee any concerns with this:

$sql = $pdo->query("SELECT sets_id, sets_nameunique FROM tableSets ORDER BY sets_nameunique;"); 
$setsQuery = $sql;
     while($row = $setsQuery->fetch()) {
          ?>
          <option value="<?php echo $row["sets_id"] ?>"<?php if ($row["sets_id"] == $setSelection) { ?> selected<?php } ?>><?php echo $row["sets_nameunique"] ?></option>
          <?php
      }

#8

Question #2: What the heck am I not understanding? Why is this not working?

$stmt = $pdo->prepare('SELECT * FROM tableMinis WHERE mini_sets_id = :selection');
$stmt->execute(['selection' => $userSelection]);
$minisQueryBySet= $stmt;

while($row = $minisQueryBySet->fetch()) {
}


#9

Since you didn’t proved any information about what result you did get, we cannot tell you what’s wrong.

You are however not doing what two different people have suggested/shown an example of, and that is separating the different concerns in the code. You need to fetch any data from the query into an appropriately named php variable and you need to stop creating unnecessary variables and copying data from one variable to another.


#10

This is the difference between learning the meaning of what you are doing, so that you can use it correctly, and just following along and mimicking something you saw. The prepare() method call returns a PDOStatement object. The execute() and fetch() methods are PDOStatement object methods. In order to call the fetch() method, you must be able to reference the PDOStatement object, but since you haven’t assigned it to a variable, it is not available and the variable you are using doesn’t contain the PDOStatement object from the prepare() call. It contains the sql query statement, a string, from where you assigned that string to a poorly named variable.

You should NOT run queries inside of loops. it is very inefficient due to the communications that take place between php and the database server. If you are querying for related data, you should use one JOIN query. If this data is for something like building select/option menus, making use of the separation of concerns design pattern will eliminate the problem since any independent queries would be groped together before the start of the html document and any data needed when producing the html document would exist in php variables.

They don’t have any special meaning, but since the problem in your code is because you didn’t use separate and appropriately named variables, you need to do something that will result in a consistent pattern that will help you to write code that works or even code where you can see what’s wrong with it.

That’s correct. Name the PDOStatement object $stmt or a similar name.

I stated to fetch the data into an appropriately named variable. The code leading up to fetching the data should be the same/similar for all queries and should NOT require a lot of unique typing to produce (an advanced next step up would be to extend the PDO class with your own database class so that you can call a single common method for both a prepared and a non-prepared query, thereby simplifying your main application code.)

You have or had a question about using fetch() or fetchAll(). For the separation of concerns design pattern, the answer is easy. If the query will at most match one row, i.e. you are finding information using a unique id, use the fetch() method. If the query will match a set of one or more rows, use the fetchAll() method. If you reach the point of extending the PDO class with your own database class, you would add two new methods that would 1) run a query and fetch a single row and 2) run a query and fetch all the rows, allowing a further reduction in your main application code.


#11

Unfortunately, different people learn different ways. In my later years in life I have become a visual learner. I have also always been a person who learns better with a problem solving approach (applied learning). It’s great that some people can read 100 pages of a programming language book, retain the information, and then go on to code. I am not one of them. For years I have taught myself coding by:

  1. Reading specific chapters that relate to what I want to do.
  2. I then type the code from the chapter examples and learn how it works.
  3. I reference online sources like w3schools to see if they have any other tidbits of information I need to know.
  4. I create my code and make regular references back to the book, and to the example codes.

The problem with trying to learn PDO MySQL, is that the very smart people / engineers that are creating the manuals completely loose sight that the people reading them will be from a variety of backgrounds – not everyone reading the manual will be an engineer. (Many times it almost appears to me that its on purpose, almost as if they are saying “hey if you haven’t gone to school to get a computer science degree, and spent years of your life coding, then this manual is not for you”.)
There is a lack of informative PDO MySQL sources out there, and the few that are just very briefly cover the specific subjects, such as the phpdelusions site. (Although its better than most). If anyone knows of a good book containing exercises and examples, I would appreciate a link for it.

While it may appear to you that I am just “mimicking what I saw”, nothing more could be more untrue. I learn by looking at the code, and then picking it apart to understand the flow. The issue I have been having, is that people have suggested several different ways to achieve what should be the same result – and the result is not always consistent, so this has created great confusion for me in learning what the code is doing.

Sorry if it took this long to explain all this, but I just want you to understand my learning style now. (Sure there was a time I could read several chapters and start coding right away, but those days are long gone).

I am greatly appreciative of your time (and the time of others that have been helping me in this thread and others) which you have spent trying to educate me through my frustration. Your method of learning and instructing is very different from my learning method, so it will take a little extra time for me to process what you are trying to convey – however it is valued greatly.

Without examples, it is hard for me to understand the above, so lets see if I can work through a line of code and relate it to what you have said above:

$sql = 'SELECT * FROM tableMinis WHERE mini_sets_id = ?';

Step 1: I am assigning a string to the variable $sql

$stmt = $pdo->prepare($sql);

Step 2: The prepare method is converting the $sql string variable into a statement object.

Step 3: $pdo stuff happens
I’m not sure what the “$pdo->” does though.

I know that this is the database connection, and it is being assigned to an object stored in the variable $pdo: $pdo = new PDO($dsn, $username, $password, $options);

I know an important part of that object is the $options:

$options = [
PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
PDO::ATTR_EMULATE_PREPARES => false,
];

Regardless, at this step the statement object created by prepare($sql) is somehow associated with the object containing the database connection info.

Step 4: The result of the statement object and it’s association with the database is then assigned to the $stmt variable name.

$stmt->execute([$setSelection]); 

Step 5: The execute method references the object in $stmt and replaces the first instance of “?” in the store object to the first variable instance in the execute method, which in this case is the $setSelection variable.

$minisQueryBySet = $stmt->fetchAll();

Step 6: The fetchAll method is applied against the statement object in $stmt

Step 7: The above assigns the result to $minisQueryBySet. The result which is being applied, is an array of all data retreived from the query stored in the $sql.

foreach ($minisQueryBySet as $minisQueryRow) {

Step 8: For every set of data in the $minisQueryBySet array, the set will be assigned a value of $minisQueryRow which will allow the ability reference that specific set of data (i.e. $minisQueryRow[‘mini_id’] for the current instance)

Now, lets work through this with my bad code example (I’m not trying to correct the bad code, as I am now using the code above for queries, but I’m just trying to figure out where I went wrong):

$result2 = 'SELECT * FROM tableMinis WHERE mini_set_id = :selection;';
$pdo->prepare($result2)->execute(['selection'=>$setSelection]); 
while($row2 = $result2->fetch()) {

So if I am understanding this right, while this line is correct: ($pdo->prepare($result2)->execute([‘selection’=>$setSelection]); ), its not outputing the result anywhere, so it should have been assigned to a variable: $stmt = $pdo->prepare($result2)->execute([‘selection’=>$setSelection]);

My while loop should have have been:
while ($row2 = $stmt - > fetch()) {

The working code would have been:

$result2 = 'SELECT * FROM tableMinis WHERE mini_set_id = :selection;';
$stmt = $pdo->prepare($result2)->execute(['selection'=>$setSelection]); 
while($row2 = $stmt->fetch()) {

Which would have been clearer if it was:

$sql = 'SELECT * FROM tableMinis WHERE mini_set_id = :selection;';
$stmt = $pdo->prepare($sql)->execute(['selection'=>$setSelection]); 
$minisQuery = $stmt;
while ($minisQueryRow = $minisQuery->fetch()) {

(Note: as noted above I don’t plan to use this chained code – I just wanted to ensure I am understanding everything correctly, and I understand better by looking at code examples which I can analyze the flow of, rather than by reading descriptive text).


#12

The php.net documentation is the defining reference manual for the php language. Likewise, the MySQL.com documentation is the defining reference manual for the sql that the MySQL database implements. These reference manuals list the language constructs, operators, and the built-in functions and classes. Their purpose is not to teach how to program or how to write sql queries. For the functions and classes, the documentation defines what call-time parameters there are, what processing is done, and what result is returned. This is how you would learn what methods the PDO class has, such as query() and prepare(), and what methods the PDOStatment class has, such as execute(), fetch(), and fetchAll(), and how to use the built-in methods and functions you have chosen based on what you are trying to accomplish.

Yes, but Keep It Simple (KISS.)

Within the context of the code that foreach() loop is part of, everything is with respect to the minis query data. There’s no need for a verbose variable named $miniQueryRow. Just use $row. We are trying to save on typing, reduce the chance of carpal tunnel syndrome, and typo mistakes that have to be found and fixed.

Yes, that should work.

Yes and No. The use of the $sql variable name makes it clearer (one point of well written code is it is self documenting and variable names should indicate the meaning of the data in the variable…) However, copying one variable to another is a waste of typing and processing time, which for the posted code should go away when you fetch the data from the query into an appropriately named variable.