-8

I don't know why this piece of code did not work, when I pass value through input text?if i am violating the rules then what is it,s correct format.

<?php        

$id1 = $_POST["id1"];  
           $name = $_POST["name"];   
           $update = $_POST["update"];   
             echo $id1;    //working
             echo $name;    //working
             echo $update;    //working



  mysqli_query($conn , 'update insert1 set  '.$name.' = '.$update.'
    where id-1 = '.$id1.'' ); //not working
        // but if I manually use this as follows it works correctly
         mysqli_query($conn , 'update insert1 set  name = "new" where id-1 = '1'' );

  ?>
Gul Amed
  • 11
  • 2
  • You need to use **$id1** in the SQL – user2182349 Jul 23 '16 at 21:00
  • 3
    You're missing quotes around your string values and you are wide open to sql injection – John Conde Jul 23 '16 at 21:00
  • i mean that it does not update the values which i want to update. – Gul Amed Jul 23 '16 at 21:06
  • Interesting indentation scheme you are using – Jonathan Jul 23 '16 at 21:09
  • If you insert comments into code for the purposes of writing a question, please ensure the new code will run. It is best to do this in your editor and then run it one more time, to ensure you are showing us something that will actually compile. – halfer Jul 23 '16 at 21:10
  • Why not just [prepare](http://php.net/manual/en/mysqli.prepare.php) / [bind](http://php.net/manual/en/mysqli-stmt.bind-param.php) / [execute](http://php.net/manual/en/mysqli-stmt.execute.php)? – Dave Chen Jul 23 '16 at 21:11
  • `'update insert1 set name = "new" where id-1 = '1''` works? I was pretty sure `-` was not a valid unquoted column character. Also the quoting there suggests that wont work. – chris85 Jul 23 '16 at 21:18
  • @chris85 probably this one is an error with copy and paste (not that the code itself isn't filled with flaws) – offchance Jul 23 '16 at 21:24

3 Answers3

1

I'd wrap $update with single quotes (notice that I flipped the quotations) and changed id1 into $id1:

mysqli_query($conn , "update insert1 set  ".$name." = '".$update."'
where id-1 = ".$id1 );

If id-1 is a string column type in the database then I'd wrap $id1 with single quotes. like this:

mysqli_query($conn , "update insert1 set  ".$name." = '".$update."'
where id-1 = '".$id1."'" );

Notes:

  • I'd double check if id-1 is intended in the WHERE condition, because it checks if the value in that column is 2 rather than 1. WHERE id - 1 = 1 is equivalent to WHERE id = 2 but more confusing to the reader (thanks to FirstOne for pointing that out).
  • As mentioned in another answer, your code is vulnerable for SQL injection, I'd check this: https://stackoverflow.com/a/16282269/4283725
Community
  • 1
  • 1
offchance
  • 642
  • 7
  • 13
  • 1
    The `where id-1 = something` means _if the **value** from id minus 1 equals to something_. Take a look at an example: [http://sqlfiddle.com/#!9/2b720/3](http://sqlfiddle.com/#!9/2b720/3) – FirstOne Jul 23 '16 at 21:35
0

First of all, please indent your code properly.

Then learn (or at least try to understand) how strings concatenation works. In PHP you can use single or double quotes for strings.

What I repeat everytime to my colleague is to try (if possible) to wrap a string into the right type of quote regarding what the string can (possibly) contain.

If you have a chance to have a single quote in your string (or in one of the variables concatenated into), wrap it into doubles.

If you have a chance to have a double quote in your string (or in one of the variables concatenated into), wrap it into singles.

This can seems obvious but if you keep that in mind every time you manipulate strings, you'll be on the way for well concatenating you strings AND variables.

Also way you are passing a full raw query as a parameter is not very readable. Put in in a separate variable and try like that :

<?php        
$id1 = $_POST["id1"];  
$name = $_POST["name"];   
$update = $_POST["update"];   

$query = '
    UPDATE insert1
    SET ' . $name . ' = "' . $update . '"
    WHERE id-1 = ' . $id1 . '
';

mysqli_query($conn, $query);
?>

You will notice that $name is not surrounded with quotes as it's a field name and not a value.

Again $id1 is not surrounded with quotes as it's an integer value and not a string value. But if for some reason the id-1 field or your insert1 table stores numbers AS strings so you'll want to surround it with double quotes.

TwystO
  • 2,456
  • 2
  • 22
  • 28
-1

It appears you are not declaring a variable in your SQL statement.

Adding the dollar sign($) to declare that.

$id1 = $_POST["id1"];  
               $name = $_POST["name"];   
               $update = $_POST["update"];   
                 echo $id1;    //working
                 echo $name;    //working
                 echo $update;    //working



      mysqli_query($conn , 'update insert1 set  '.$name.' = '.$update.'
        where id-1 = '.$id1.'' ); //not working
            //but  if i manually use this as folwing it works correctly    mysqli_query($conn , 'update insert1 set  name = "new" where id-1 =
        '1'' );

  ?>

Also, your code is open for SQL injection.

ahinkle
  • 2,117
  • 3
  • 29
  • 58
  • Words of wisdom there. Maybe you can explain *why* its open to SQL injection? Too often I see people drop-in someone else's observations into their answer... without actually understanding it themselves. – Jonathan Jul 23 '16 at 21:12