0

I need to check if a form input is set and run different queries based on that. All the queries are to the same DB and most of them are to the same table. Here is an example: (Note this is a simplified code for an example)

//First query

if (isset ($_POST['abccheckbox']))
{

    $abccheckbox = implode("','", $_POST['abccheckbox']); 

    $connect = mysqli_connect("localhost", "root", ""); 
    if (!$connect) { 
        die('Connection Failed: ' . mysqli_error()); 
    } 

    //Connect to Database   
    mysqli_select_db($connect, "trialDB") or die( "Unable to select database");

    $query= "SELECT * FROM xyz_table WHERE xyz IN ('$abccheckbox')";  
    $result=mysqli_query($connect, $query);

    if (!$result)  
    {  
      die('Error fetching results: ' . mysqli_error()); 
      exit();  
    }


    while ($row = mysqli_fetch_array($result))  
    {

        $description[] = $row['description'];  
    }  

    //Closing DB Connection
    mysqli_close($connect); 

}

else {
        $description = 0;
}


//SECOND QUERY

if (isset ($_POST['defcheckbox']))
{

    $defcheckbox = implode("','", $_POST['defcheckbox']); 

    $connect = mysqli_connect("localhost", "root", ""); 
    if (!$connect) { 
        die('Connection Failed: ' . mysqli_error()); 
    } 

    //Connect to Database   
    mysqli_select_db($connect, "trialDB") or die( "Unable to select database");

    $query= "SELECT * FROM xyz_table WHERE xyz IN ('$defcheckbox')";  
    $result=mysqli_query($connect, $query);

    if (!$result)  
    {  
      die('Error fetching results: ' . mysqli_error()); 
      exit();  
    }


    while ($row = mysqli_fetch_array($result))  
    {

        $headings[] = $row['headings'];  
    }  

    //Closing DB Connection
    mysqli_close($connect); 

}

else {
        $headings = 0;
}


//Third Query

if (isset ($_POST['ghicheckbox']))
{

    $ghicheckbox = implode("','", $_POST['ghicheckbox']); 

    $connect = mysqli_connect("localhost", "root", ""); 
    if (!$connect) { 
        die('Connection Failed: ' . mysqli_error()); 
    } 

    //Connect to Database   
    mysqli_select_db($connect, "trialDB") or die( "Unable to select database");

    $query= "SELECT * FROM xyz_table WHERE xyz IN ('$ghicheckbox')";  
    $result=mysqli_query($connect, $query);

    if (!$result)  
    {  
      die('Error fetching results: ' . mysqli_error()); 
      exit();  
    }


    while ($row = mysqli_fetch_array($result))  
    {

        $type[] = $row['type'];  
    }  

    //Closing DB Connection
    mysqli_close($connect); 

}

else {
        $headings = 0;
}


......

I am bascially trying to query the same table but doing different queries and storing different values in array after checking if that particular set of checkboxes are selected. I feel that the code is really windy and very redundant.

My questions are:

1) Is it fine to have multiple SELECT queries to SQL in 1 file itself or having too many separate queries in 1 file can overload the server?

2) In the above code, I open sql connection and close it for each queries. Does that affect the performance if I connect to sql so many times and run separate queries in the same file?

3) Can someone give an example on how I can maybe simply that code for efficiency so I can understand the right way of doing it please?

Neel
  • 9,352
  • 23
  • 87
  • 128

2 Answers2

1
  1. (My)SQL is specifically designed to deal with enormous amount of queries that might overload the server, but nevertheless 1 query is really not enough to make it happen. You'd need like 100 queries only to slow a little the request, but it's unlikely that your website is going to hang so much due to MySQL queries (unless you select an entire table of 100 thousands of rows 1000 times).

  2. Why do you need to close the connection after you've queried the database? Just execute both queries and then close the database. It's going to take also less time because if the MySQL server is remote (not part of your machine) the connection is going to take a bit.

  3. Your code, from my point of view, is not that bad, however your queries might have issues with SQL injection. Read this for more information about it. Lastly, just follow what I have suggested you the previous answers and you'll be fine.

Something like this I think would be better:

$connect = mysqli_connect("localhost", "root", "");

if(!$connect) 
{
    die('Connection Failed: ' . mysqli_error()); 
}

// Connect to the database.
mysqli_select_db($connect, "trialDB") or die( "Unable to select database"); 

if(isset($_POST['abccheckbox']) || isset($_POST['defcheckbox']) || isset($_POST['ghicheckbox']))
{   
    $things = array();

    // The variable is now also query safe.
    if(isset($_POST['abccheckbox'])) 
    {
        $things['abccheckbox'] = mysqli_real_escape_string($connect, implode("','", $_POST['abccheckbox']));
        $result = mysqli_query($connect, "SELECT * FROM xyz_table WHERE xyz IN ('{$things['abccheckbox']}')");

        // If there's no result we show an echo but we do not interrupt the page from loading as we need to load other queries afterwards.
        if (!$result)  
        {       
            echo "Error fetching results: " . mysqli_error(); 
            $description = 0;
        }
        else
        {
            while ($row = mysqli_fetch_array($result))  
            {
                $description[] = $row['description'];  
            }
        }
    }

    if(isset($_POST['defcheckbox'])) 
    {   
        $things['defcheckbox'] =  mysqli_real_escape_string($connect, implode("','", $_POST['defcheckbox']));

        $result = mysqli_query($connect, "SELECT * FROM xyz_table WHERE xyz IN ('{$things['defcheckbox']}')");

        if (!$result)  
        {       
            echo "Error fetching results: " . mysqli_error(); 
            $headings = 0;
        }
        else
        {
            while ($row = mysqli_fetch_array($result))  
            {
                $headings[] = $row['heading'];  
            }
        }
    }

    if(isset($_POST['ghicheckbox'])) 
    {   
        $things['ghicheckbox'] =  mysqli_real_escape_string($connect, implode("','", $_POST['ghicheckbox']));

        $result = mysqli_query($connect, "SELECT * FROM xyz_table WHERE xyz IN ('{$things['ghicheckbox']}')");

        if (!$result)  
        {       
            echo "Error fetching results: " . mysqli_error(); 
            $types = 0;
        }
        else
        {
            while ($row = mysqli_fetch_array($result))  
            {
                $types[] = $row['type'];  
            }
        }
    }
}

mysqli_close($connect);
Community
  • 1
  • 1
GiamPy
  • 3,543
  • 3
  • 30
  • 51
  • @MarcelKorpel: Thank you for reminding me that, added in the answer. – GiamPy Jan 05 '14 at 16:51
  • Thank you so much for clarifying this for me. I was just not sure about having multiple queries and your answers helped me understand on that. I have taken the note on SQL injection although I didint show that in the sample code above. So I think I might have the code like this: Connect to Database -> Run the check for isset for each case -> Run the Query inside each if isset() statement is successful -> Do the same for each case -> Close the sql connection last. Thanks for your example. – Neel Jan 05 '14 at 17:19
  • If I am querying the same sql table would it make a difference if I (a) Downloaded the entire table first and save it in an `$result` array by using only 1 query and then use the `while ($row = mysqli_fetch_array($result))` check for each cases without needing to query separately -or- (b) fetch just the table rows for the values needed for each case separately like the example above by having separate queries and then use the `while ($row = mysqli_fetch_array($result))` to check with that particular array? Does (a) and (b) make any difference? Table is about 8 columns and 100-150 rows only. – Neel Jan 05 '14 at 17:28
  • I would prefer just making a single query to the entire table (if it's necessary - I always prefer to select only what you really need instead of querying the entire table) and then use the values you need instead of multiple queries. – GiamPy Jan 05 '14 at 17:54
  • Thank you GiamPy. I was just trying your example. I love your example heaps better than what I had previously. However, I am receiving one error: mysqli_real_escape_string() expects exactly 2 parameters, 1 given `$things['abccheckbox'] = mysqli_real_escape_string(implode("','", $_POST['abccheckbox']));` – Neel Jan 05 '14 at 18:00
  • Sorry, I forgot to add the connection link. It should be `mysqli_real_escape_string($connect, implode("','", $_POST['abccheckbox']))`. – GiamPy Jan 05 '14 at 18:01
  • @blackops_programmer: No problem! Have fun! – GiamPy Jan 05 '14 at 18:07
1

I didn't read the code, just the prose. I am not a php programmer. Having said that, I'll say this.

For question 1, if php is anything like .net or ColdFusion, then each set of query results returned to your application are stored in the RAM of your application server. On busy servers, that could be problematic.

You stated, "I am bascially trying to query the same table but doing different queries and storing different values in array after checking if that particular set of checkboxes are selected.".

That approach seems inefficient. You might be better off doing your logic on the checkboxes before you visit the database. In fact, under certain circumstances, running a query might not be necessary.

Dan Bracuk
  • 20,699
  • 4
  • 26
  • 43