1

I'm querying a database based on variables and I'm a bit confused about how to get my function is not working. If there is only one string in the SQL criteria array, I want it to say WHERE and if there is more then one, I want the first one to be WHERE and the rest to be AND. Currently if I have one string and the rets are blank then this what the query looks like: SELECT * FROM tb_visitors AND date BETWEEN '2017-08-01' AND '2017-08-31'. I want it to be WHERE first, is there a better way to do this?

        $sites =  $data['sites'];
        $attendence =  $data['filter'];
        $time = $data['time'];
        $department = $data['department'];
        $staff = $data['staff'];

        $sql = 'SELECT * FROM tb_visitors' ;
        $sql_criteria= "";

        if(is_numeric($department) == true){
           $sql_criteria[] = "dept_id = "."'".$department."'"; 
        } else if($staff !== 'Staff Member') {
             $sql_criteria[] = 'staff='.'"'.$staff.'"'; 
        } 

        if($department == 'Department' || $staff == 'Staff Member' ){
           $sql_criteria[] = ""; 
        }

        // Time
        if($time == 'Date Range' || $time == 'Custom' || $time == 'Today'){
           $sql_criteria[] = ""; 
        } else if($time == 'Today'){
           $today_date = date("Y-m-d"); 
           $sql_criteria[] = "date ="."'".$today_date."'";  
        } else if($time == 'Tomorrow'){
           $d= strtotime("tomorrow");
           $date =  date("Y-m-d", $d);
           $sql_criteria[] = "date ="."'".$date."'"; 
        }  else if($time == 'Yesterday'){
           $date = date('Y-m-d',strtotime("-1 days"));
           $sql_criteria[] = "date ="."'".$date."'"; 
        } else if($time == 'Last Month'){
           $start_date =  date("Y-n-j", strtotime("first day of previous month"));
            $end_date =  date("Y-n-j", strtotime("last day of previous month"));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'"; 
        } else if($time == 'Next Month'){
           $start_date =  date("Y-n-j", strtotime("first day of next month"));
           $end_date =  date("Y-n-j", strtotime("last day of next month"));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'"; 
        } else if($time == 'This Week'){
           $start_date = date("Y-m-d", strtotime('monday this week'));
           $end_date =  date("Y-m-d", strtotime('sunday this week'));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'";  
        } else if($time == 'This Month'){
           $start_date = date('Y-m-01');
            $end_date = date('Y-m-t');
            $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'";
        } else if($time == 'Last Week'){
           $start_date = date("Y-m-d", strtotime("last week monday"));
           $end_date =  date("Y-m-d", strtotime("last week sunday"));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'"; 
        } else if($time == 'Next Week'){
           $start_date = date("Y-m-d", strtotime("next monday"));
           $end_date =    date("Y-m-d", strtotime('+1 week sunday'));
           $sql_criteria[] = "date BETWEEN '".$start_date."' AND '".$end_date."'";  

        } 

        // Attendence 
        if($attendence == 'Status' || $attendence == 'All Visits'){
            $sql_criteria[] = ""; 
        } else if($attendence == 'No Show'){
            $sql_criteria[] = "no_show = 'Y'";  
        } else if($attendence == 'Completed Visits'){
            $sql_criteria[] = "seen IS NOT NULL";  
        } else if($attendence == 'Upcoming Visits'){
           $today_date = date("Y-m-d"); 
           $sql_criteria[] = "date > '".$today_date."'"; 
        } 

        //Sites

        if($sites == 'All Sites' || $sites == 'Sites' ){
            $sql_criteria[] = ""; 
        } else if($sites == 'Twickenham'){
            $sql_criteria[] = "dept_id NOT IN ('28',  '29',  '30',  '32',  '41',  '44')";
        } else if($sites == 'Hammersmith'){
            $sql_criteria[] = "dept_id IN ('36')"; 
        } else if($sites == 'Heatherwood'){
           $sql_criteria[] = "dept_id IN ('37')";  
        } else if($sites == 'Hillingdon'){
          $sql_criteria[] = "dept_id IN ('38')";   
        } else if($sites == 'Nuffeild'){
            $sql_criteria[] = "dept_id IN ('39')"; 
        } else if($sites == 'St Georges'){
            $sql_criteria[] = "dept_id IN ('41')"; 
        } else if($sites == 'Queen Victoria'){
            $sql_criteria[] = "dept_id IN ('40')"; 
        } else if($sites == 'Stoke Mandeville'){
           $sql_criteria[] = "dept_id IN ('42')";  
        }


    $i=0;
    foreach ($sql_criteria as $a => $b){

       if($b == ""){
            $join = "";   
        } else if($i == 0){
            $join = "WHERE";
        } else {
            $join = "AND";  
        }


    $sql_final = $join." ".$b;
    $i++;
    $sql = $sql." ".$sql_final;
    }
   $result = $db->db_num("$sql");
   echo $sql;
4
  • 4
    this is horrendously vulnerable to injection attacks. You need to sanitise your inputs really carefully, and use parameters wherever possible. For anything that can't be parameterised, use whitelisting. Commented Aug 29, 2017 at 15:12
  • 2
    @user3053151, I highly recommend you look at this documentation for php prepared statements php.net/manual/en/class.pdostatement.php Commented Aug 29, 2017 at 15:20
  • Hey, Thanks for the advice, don't worry, this project is not going to go online, its for a private visitors book I'm trying to make. Thanks for the link, I'll have a read. Commented Aug 29, 2017 at 15:25
  • it'll have to go online somewhere if people are going to use it (unless you mean you will be the only user, on localhost). Even if it's on a local network / intranet, you can't make any assumptions about who / what is going to try and send requests to it, and what their intentions are. Plus it's good practise for the future. Commented Aug 29, 2017 at 16:07

1 Answer 1

2

Since you've already got your where clauses in an array, you can utilize count and implode.

$sql = 'SELECT * FROM tb_visitors';
$sql_criteria = array();

/*
* Build the $sql_criteria array
* [...]
*/

if (count($sql_criteria)) // Should the where clause be built?
{
    $sql .= " WHERE " . implode(" AND ", $sql_criteria);
}

Note: As a commenter has already stated, you're vulnerable to SQL injections. This is important to consider, because injections can occur "accidentally", and are not always due to a targeted attack. For example, if your $staff had a value of "John O'Sullivan", the single quote in the name would break your query.

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

3 Comments

With this I now get the follow if one of the variables are an empty string: "SELECT * FROM tb_visitors WHERE AND AND AND dept_id IN ('36')
@user3053151 The simple answer is to not have an empty string, haha. But if you want to keep an empty string in your array, you can sanitize the $sql_criteria through array_filter() to remove the blank values. If you intend to keep empty strings in your $sql_criteria, I can update the answer to demonstrate that. Edit: this question is about removing blanks from an array.
I got rid of the empty strings, I think you have cracked the case :) Many thanks!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.