SQL Statement within nested while loops

Question

I'm writing up a stock take application for our IT department and I'm not confident the way I'm going is the best.

I have created a 'productgroup' table and a 'products' table that are linked with an ID (one productgroup to many products) product group for example LaserJet Pro 400 and products would be the individual consumables.. black, magenta, cyan etc.

So what is happening is I have a while loop for displayting the groups and then a nested while loop for displaying the products within that group.

What I was worried about is it being a lot of sql statements in rapid fire, there doesn't seem to be an issue with performance at this early stage but I'm unsure.. is this acceptable? is there a better way?

foreach ($_POST['BeginLocation'] as $key => $value) {$LocationID = $key;}

echo '<div class="nmform"><form name="StockTake" action="index.php" method="post" enctype="multipart/form-data">';

include "../DBCon/RDPNearMisses.php";

$GetProductGroups = sqlsrv_query($NMDB, "select distinct PD.ProductGroupID, PG.GroupName
                                        from [JobObservations].[dbo].[ITStk.Products] as PD
                                        inner join [JobObservations].[dbo].[ITStk.ProductGroups] as PG on PD.ProductGroupID = PG.ProductGroupID
                                        where PD.LocationID = $LocationID");

                       if( $GetProductGroups === false ) {
                           if( ($errors = sqlsrv_errors() ) != null) {
                               foreach( $errors as $error ) {
                                   echo "SQLSTATE: ".$error[ 'SQLSTATE']."<br />";
                                   echo "code: ".$error[ 'code']."<br />";
                                   echo "message: ".$error[ 'message']."<br />";
                               }
                           }
                       }

                       while ($row = sqlsrv_fetch_array($GetProductGroups)) {echo '<h4>'.$row['GroupName'].'</h4>';
                                                $ProductGroupID = $row['ProductGroupID'];

                                                $GetProducts = sqlsrv_query($NMDB, "select PD.ProductID, PD.ProductGroupID, PD.ProductCode, PD.ProductDescription
                                                from [JobObservations].[dbo].[ITStk.Products] as PD
                                                inner join [JobObservations].[dbo].[ITStk.ProductGroups] as PG on PD.ProductGroupID = PG.ProductGroupID
                                                inner join [JobObservations].[dbo].[ITStk.Locations] as LC on PD.LocationID = LC.LocationID
                                                where PD.LocationID = $LocationID
                                                and PD.ProductGroupID = $ProductGroupID
                                                order by LC.LocationDescription asc, PG.GroupName asc, PD.ProductCode asc");

                                                if( $GetProducts === false ) {
                                                    if( ($errors = sqlsrv_errors() ) != null) {
                                                        foreach( $errors as $error ) {
                                                            echo "SQLSTATE: ".$error[ 'SQLSTATE']."<br />";
                                                            echo "code: ".$error[ 'code']."<br />";
                                                            echo "message: ".$error[ 'message']."<br />";
                                                        }
                                                    }
                                                }
                                                echo '<table><th>Code</th><th>Description</th><th>Qty</th>';
                                                while ($row1= sqlsrv_fetch_array($GetProducts)) {echo '<tr><td>'.$row1['ProductCode'].'</td><td>'.$row1['ProductDescription'].'</td><td><select name="'.$row1['ProductID'].'"><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option></select></td></tr>';}
                                                echo '</table>';


                       }
                       echo '<input type="Submit" name="SubmitStock"></form>';
                       sqlsrv_close($NMDB);
         echo '</div>';

There are many issues with the code here. First is you need to read about, understand and start using parameterized queries. Second is you could probably turn this whole nested loop solution into a single query. If you really want some help with this please read this article. spaghettidba.com/2015/04/24/… — Sean Lange
– Sean Lange, Commented Sep 9, 2016 at 14:18
I could easily turn it into a single query from a SQL point of view, the problem is how i present it using php with a groupname heading then subproducts. — David H
– David H, Commented Sep 9, 2016 at 15:04
Sean, thanks for the mention of parameterized queries. My environment is pretty low risk to SQL injection due to not being web facing but I will adopt that. Unfortunately I'm Google taught on PHP so I still come across a lot that is basic to others and news to me. — David H
– David H, Commented Sep 9, 2016 at 15:18
Certainly don't make the mistake of thinking that your application is safe because it isn't web facing. It doesn't take much for that to change. And internal applications are not immune to sql injection, just a lower probability. Also if you refer to your old code when making a web application it is easy to copy the style you used previously. I can't help you with the php side of things as I can't even spell it correctly. :D — Sean Lange
– Sean Lange, Commented Sep 9, 2016 at 15:31

JNevill · Accepted Answer · 2016-09-09 15:36:26Z

1

While your data is small and the number of groups per location is small, you probably won't notice a difference, but as the number of groups per location changes you may want to consider switching to 1 query and a while loop:

SELECT DISTINCT
    PD.ProductID,
    PG.GroupName,
    PD.ProductGroupID,  
    PD.ProductCode,
    PD.ProductDescription
FROM [JobObservations].[dbo].[ITStk.Products] AS PD
INNER JOIN [JobObservations].[dbo].[ITStk.ProductGroups] AS PG ON PD.ProductGroupID = PG.ProductGroupID
INNER JOIN [JobObservations].[dbo].[ITStk.Locations] AS LC ON PD.LocationID = LC.LocationID
WHERE PD.LocationID = $LOCATIONID;

Would work well since it gets you a record set containing your unused GroupName and the Product ID and Produce Description that you are using in your table.

Furthermore unless the table ITStk.Locations is being joined in merely to limit records where a LocationID is actually in the Locations table, then there is no need for it to be joined here. You don't use any of the fields to limit result sets or SELECT.

Nearly any time you find yourself grabbing a recordset and while reading/looping the record set you are issuing more SQL, you can turn that into a single SQL statement.

To get your GroupName as a header over your tables, in your 1 while loop you could do something like the following (Forgive me, it's been a few years since I wrote PHP):

$GetProductGroups = sqlsrv_query($NMDB, "SELECT DISTINCT
                                        PD.ProductID,
                                        PG.GroupName,
                                        PD.ProductGroupID,  
                                        PD.ProductCode,
                                        PD.ProductDescription
                                    FROM [JobObservations].[dbo].[ITStk.Products] AS PD
                                    INNER JOIN [JobObservations].[dbo].[ITStk.ProductGroups] AS PG ON PD.ProductGroupID = PG.ProductGroupID
                                    INNER JOIN [JobObservations].[dbo].[ITStk.Locations] AS LC ON PD.LocationID = LC.LocationID
                                    WHERE PD.LocationID = $LOCATIONID
                                    ORDER BY GroupName;");

if( $GetProductGroups === false ) {
   if( ($errors = sqlsrv_errors() ) != null) {
       foreach( $errors as $error ) {
           echo "SQLSTATE: ".$error[ 'SQLSTATE']."<br />";
           echo "code: ".$error[ 'code']."<br />";
           echo "message: ".$error[ 'message']."<br />";
       }
   }
}

while ($row = sqlsrv_fetch_array($GetProductGroups)) {

    /*If the groupname of the current record is different then the last record's group name (notice order by on query) then echo out the header and start a new table*/
    if ( $groupname != $row['GroupName']) {
        echo '<h4>'.$row['GroupName'].'</h4>';
        echo '<table><th>Code</th><th>Description</th><th>Qty</th>';
    }

    /*echo out the row into the table*/
    echo '<tr><td>'.$row['ProductCode'].'</td><td>'.$row['ProductDescription'].'</td><td><select name="'.$row1['ProductID'].'"><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option><option>0</option></select></td></tr>';


    /*again if we started a new table because the groupname is new in the recordset, then close the table*/
    if ( $groupname != $row['GroupName']) {         
        echo '</table>';
    }
    /*Capture group name for future iterations*/    
    $groupname = $row['GroupName'];

}

echo '<input type="Submit" name="SubmitStock"></form>';
sqlsrv_close($NMDB);
echo '</div>';

edited Sep 9, 2016 at 15:36

answered Sep 9, 2016 at 14:33

JNevill

50.6k4 gold badges46 silver badges72 bronze badges

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

7 Comments

David H Over a year ago

You're right I didn't need to include the locations table as I'm only referencing location ID for filtering. The reason there is two queries the first grabs the list of possible product groups for the location ID (chosen on the previous page) and the nested while loop query grabs the products to display under the heading of each product group. I could get the same information with one query but I don't know how I would organise it on the page with GroupName being a header and a list of products underneath it as it is now. Note the GroupName is displayed before the nested while loop.

JNevill Over a year ago

You could do that in your while loop. Grab the group into a variable and, if it's the first record OR the current group is not equal to the stored group in the variable, then print a new header. That will be much faster for PHP to parse then it will be to query the database for a new recordset. Queries are very expensive, a variable and an if statement is cheap. You probably won't notice much of a difference at first, but as the number of groups per location grow, your page is going to chug more and more if you continue with 1 query per group.

JNevill Over a year ago

Oh! I see that groupname echo out now in the H4 tag. I'll edit that part of the question, but the solution still stands.

David H Over a year ago

Yes, I figured it wasn't a good method. Could you give me a dog rough example of what you mean? I'm struggling to visualize how that would look in php.

JNevill Over a year ago

I've updated the answer to include an attempt at the PHP side of this. My PHP is rusty as hell, but I think this is very close. Basically the table tag is opened and closed depending on whether or not we've encountered a new groupname in the recordset as we are iterating. I've included an ORDER BY clause in the SQL statement in that PHP snippet so that distinct groupnames will be encountered as the iteration occurs.

|

Collectives™ on Stack Overflow

SQL Statement within nested while loops

1 Answer 1

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related