0

The below code displays data from a table and then filters it depending on the results of two combo boxes. I am able to order the results by ID once the form is submitted, but not on initial load (where all are listed). I have tried $sql = "SELECT * FROM Places ORDER BY ID"; which works when the list loads but returns an error when the form is submitted. Hope that makes sense. Any ideas? Thanks!

// Default query
$sql = "SELECT * FROM Places";
// check if form was submitted
if (isset($_POST['area'])) {
    $connector = 'where';
    if ($_POST['area'] != 'All') { 
        $sql .= " where Area = '".$_POST['area']."' ORDER BY ID";
        $connector = 'and';
    }
    if ($_POST['theme'] != 'All') { 
        $sql .= " $connector Theme = '".$_POST['theme']."' OR Theme2 = '".$_POST['theme']."' 
        ORDER BY ID";
    }
}
cbladon
  • 1
  • 2
  • 1
    The `ORDER BY` approach is the right one, but you need to tell us more about the "error when the form is submitted". Please show us your form as well as the code it is submitted to. – RandomSeed Jul 17 '13 at 15:42
  • Hi, the error was 'mysql_fetch_array() expects parameter 1 to be resource, boolean given...' – cbladon Jul 17 '13 at 15:45
  • - if that's any help, code generated by web package – cbladon Jul 17 '13 at 15:46
  • you're generating invalid sql, causing the query to fail. probably that dangling `and` in the area!=all code section. You also have gaping wide open [SQL injection attack](http://bobby-tables.com) vulnerabilities, so enjoy having your server pwn3d. – Marc B Jul 17 '13 at 15:47
  • Sorry I didn't get it that this code is actually the one the form is posted to. You probably have a SQL syntax error in your generated query. Please show us the contents of `$sql` just before you send it to MySQL. – RandomSeed Jul 17 '13 at 15:47
  • I think this is just it, sorry am new to this, edited the code someone else helped me with so not really sure what I'm doing! This is the code directly after the above - //execute the SQL query and return records $result = mysql_query($sql); ?> – cbladon Jul 17 '13 at 15:50

4 Answers4

0

Your ORDER BY ID clause must appear at the very end of your statement. If both $_POST['area'] and $_POST['theme'] are filled, you end up with a query like this:

SELECT ... WHERE Area = 'some area' ORDER BY ID AND Theme = 'some theme'

Add the ORDER BY bit as the last part of your query.

RandomSeed
  • 29,301
  • 6
  • 52
  • 87
  • Also, your code is wide open to SQL injection. Do no concatenate anything from your user input (eg. `$_POST`) with your queries. Read about "[SQL injection](http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection-in-php)". – RandomSeed Jul 17 '13 at 15:53
0

I think you are missing a default behavior statement. I.e. Your IF statement doesn't have an else clause. So you are checking for isset and if it is change the select query, but there is nothing to say IF ! isset SELECT query should be .... ORDER BY ID.

Also I would try echoing your SQL queries out each time you set / change a portion of it to understand exactly what is being sent to the DB.

Lastly I always check the mysql.general_log table for the last run queries to see what is actually happening at the DB end.

Simon
  • 238
  • 2
  • 3
  • 10
0

It looks like it is possible for $_POST['area'] != 'All' and $_POST['theme'] != 'All'. In that case you will be putting the ORDER BY clause in twice. That probably your problem.

So try this.

// Default query
$sql = "SELECT * FROM Places";
// check if form was submitted
if (isset($_POST['area'])) {
    $connector = 'where';
    if ($_POST['area'] != 'All') { 
        $sql .= " where Area = '".$_POST['area']."'";
        $connector = 'and';
    }
    if ($_POST['theme'] != 'All') { 
        $sql .= " $connector Theme = '".$_POST['theme']."' OR Theme2 = '".$_POST['theme'] . "'";
    }

    if ( $_POST['area'] != 'All' || $_POST['theme'] != 'All' ) {
        $sql .= ' ORDER BY ID';
    }
}
RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
0

Thanks for all your help, I have solved the problem at the server end anyway so no need for code. Thanks for bringing attention to the security issues, I had these in the back of my mind but wasn't sure how bad it was! If I change the code to PDO would it help greatly? I have already reduced the privileges of the user to minimal. Thanks again.

cbladon
  • 1
  • 2
  • Please edit your answer and give a short description of the solution -- it could help someone who stumbles upon this thread. Then mark your own answer as accepted so as to close the topic. If you have more questions, then... ask a new question! – RandomSeed Aug 01 '13 at 22:54