0

I am using a query to delete a row... The quotes seems fine..But am getting error.. i tried the following two ways, but it is not working..

          $sch=mysql_query("DELETE FROM schedule WHERE
    membername='$value' AND trainername='$_SESSION['username']'") or die(mysql_error());

           $sch= mysql_query("DELETE FROM schedule 
                       WHERE membername=$value 
                        AND trainername=$_SESSION['username']");

Any suggestion?? Thank You..

user2245360
  • 11
  • 1
  • 4
  • 1
    Please, read up on [proper SQL escaping](http://bobby-tables.com/) before you get in trouble. – tadman Apr 04 '13 at 19:01
  • 1
    You **absolutely** need to be escaping _every single_ value that goes to your database. Period. End of story. Whether it comes from a session or a constant defined two lines above your query. _Always_ escape. And you probably should move away from MySQL to PDO/MySQLi - since `mysql_*` functions are deprecated – Colin M Apr 04 '13 at 19:01
  • mysql_query()? are you aware of the non-depreciated method mysqli_query()? – imulsion Apr 04 '13 at 19:02
  • PDO is better than `mysqli`, and better still is using [a proper framework](http://www.phpframeworks.com/top-10-php-frameworks/) instead of smashing around with raw SQL. – tadman Apr 04 '13 at 19:05
  • 1
    Your `delete` query is perfect, can you tell me which error you are getting? – Sumit Bijvani Apr 04 '13 at 19:06
  • The only one that luckily skipped the deprecation (which is not the question) and escaping (which is the rule of thumb, in fact, but may not be the problem) is @SumitBijvani. Telling him how rude is to still use `mysql` looks like _"Why do you use a pan for making chips? You should use a deep-fryer"_. Maybe this project is years old, maybe the server will never get newer PHP versions. The tool he is using may be just fine for the case. The solution could simply go through `echoing` the query and printing `mysql_error()` to see what happened. – Sergi Juanola Apr 04 '13 at 19:19

2 Answers2

0

You need to wrap the array variable in the string with curly braces.

$test = "this is a test {$_SESSION['string']}";
Jonathan Kuhn
  • 15,279
  • 3
  • 32
  • 43
  • 1
    You need to wrap this in `mysql_real_escape_string` or it's just plain reckless. – tadman Apr 04 '13 at 19:02
  • @tadman that doesn't imply a downvote, since using `mysql_` in the first place is **bad**. – HamZa Apr 04 '13 at 19:03
  • 1
    yes, use pdo/mysqli or at least escape the string...etc. all that junk. That is why I didn't use sql in my example. This is still a problem they have with their code. – Jonathan Kuhn Apr 04 '13 at 19:04
  • Fair enough. It's still worth mentioning that this wouldn't even be a problem if SQL placeholders were used. – tadman Apr 04 '13 at 19:07
0

You can't use quoted array keys in a PHP double-quoted string. It should be 

  ... AND trainername=$_SESSION[username] ...
or
  ... AND trainername={$_SESSION['username']} ...

You've also forgotten to QUOTE those presumed-strings you're inserting into the query

... AND trainername='...'
                    ^---^--
Marc B
  • 356,200
  • 43
  • 426
  • 500
  • Don't advocate the first solution...I know it's valid when it's inside a string - but people will have a tendency to use the same notation outside of a string as well. Sadly for the future of PHP - it works...but it's bad practice. – Colin M Apr 04 '13 at 19:02
  • Neither of these is escaped correctly and won't work. `AND trainername=bob` is not valid SQL. – tadman Apr 04 '13 at 19:03
  • Both quoting *and* escaping are required for this to be valid. – tadman Apr 04 '13 at 19:04
  • It's not valid. If username is `O'Malley` then it will not work. – tadman Apr 04 '13 at 19:08
  • Plus, escaping without quoting may **still** lead to SQL injection anyway. `trainername=0 OR 1=1` (see [this answer](http://stackoverflow.com/a/5741264/616225)) – Sergi Juanola Apr 04 '13 at 19:16