2

I have large tables in my database and instead of specifying each column name I am trying to build the query dynamically.

I am trying to do an update in the 'motherboard' table based on the POST data received. The $data object i receive has more fields than the table has. (I added some fields for some flags.) Hence, I am retrieving the record I'm about to update and by comparing each of it's columns with my $data object fields I am constructing the UPDATE query.

I'm new to php, therefore I don't know the syntax well.

This is the code:

<?php
$data = json_decode($_POST["data"], true);
$id = $data["ID"];

include_once 'dbconnect.php';
$query = sprintf("SELECT * FROM `motherboard` WHERE ID = " . $id . ";");
$result = mysqli_query($con, $query);
$existingData = mysqli_fetch_assoc($result);
include_once 'dbclose.php';



$statement = "";
$statement = "UPDATE motherboard SET ";
$flag = false;
foreach ($existingData as $key => $value) {
    if ($existingData->$key != $data->$key) {
        $statement .= $key . " = " . $data->$key . " , ";
        $flag = true;
    }
}
if ($flag)
    $statement = substr($statement, 0, strrchr($statement, ',') - 1);

$statement .= " WHERE ID = " . $id . ";";

echo $statement;
?>

My main problem is in the foreach loop. I don't know how can I compare and then use for building the query the $existingData and $data variables.

How can I achieve this?

Razvan
  • 3,017
  • 1
  • 26
  • 35
  • 5
    **Danger**: You are using [an **obsolete** database API](http://stackoverflow.com/q/12859942/19068) and should use a [modern replacement](http://php.net/manual/en/mysqlinfo.api.choosing.php). You are also **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that a modern API would make it easier to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin Dec 12 '13 at 18:33
  • @Quentin .. beat me to it. – Kenny Thompson Dec 12 '13 at 18:33
  • I don't understand the need to select from DB first. If you only have certain pieces of data to update for a row, only update that data. You don't need to have every fiedl accounted for in the UPDATE statement. – Mike Brant Dec 12 '13 at 18:37
  • 1
    Your foreach loop is utterly useless - you're trying to loop on a mysqli result handle. It is **NOT** something that's iterable like that. – Marc B Dec 12 '13 at 18:37
  • @MikeBrant I want to check first if the column value is different from the post data. Only then I will add the column to the UPDATE query. – Razvan Dec 12 '13 at 18:40
  • @MarcB Otherwise I can fetch the fields from the result handle and loop the array `$array = $existingData->fetch_fields();` – Razvan Dec 12 '13 at 18:42
  • I have been bombarded with downvotes and suggestions about frameworks I could use. I am just asking if I can build that query in the way I described it??? – Razvan Dec 12 '13 at 18:47
  • I'd hardly say 2 downvotes (time of writing this) is "bombarded". The advice in answers is (in a way) answering your question about "can I do it this way?". There are many ways to do things in PHP and mysql and other languages, but the question is, are they stable, secure, clean well structured code that can be maintained and extended without a full re-write due to a bad design. The info and advice is free, take it or leave it :) – James Dec 12 '13 at 18:53
  • @razvan Why compare whether the data matches or not to determine if you add it to update statement? If the data matches it will just be updated with the matched value. Basically, you are adding an unnecessary query and looping through the results of that query to mix here when you should be able to go straight to updating the the record. Also you are currently doing nothing to sanitize teh input data, which is something you should do. – Mike Brant Dec 12 '13 at 19:01
  • @MikeBrant Not for a specific reason. Just to avoid writing a lot code. The tables have > 20-30 columns. And maybe for learning something new... – Razvan Dec 12 '13 at 19:08
  • I got it working eventually. Should I remove the question if it is downvoted? – Razvan Dec 12 '13 at 19:41
  • @Quentin I cant see any mysql_* functions. – 3ventic Dec 12 '13 at 21:39

4 Answers4

3

Don't use this approach please, if you want a SOLID application that will outrun the ages, use specific column names and not some junkish foreach loop that builds your SQL for you. If you want to evade the writting of SQL, use an ORM, there are ton's that exist out there and most of them are bundled with a framework right off the start making it simpler to learn the ropes!

Examples of simple to learn frameworks: (But not necessarely weak frameworks)

  • Cake PHP
  • Laravel

Good luck

Mathieu Dumoulin
  • 12,126
  • 7
  • 43
  • 71
0

You need to change some code ...

$result = mysqli_query($con, $query);
$existingData = mysqli_fetch_assoc($result);

Now your $existingData is an array you can loop though;

cmorrissey
  • 8,493
  • 2
  • 23
  • 27
0

Honestly I would recommend you take advantage of a framework with an ORM or just a standalone ORM. I suggest Laravel or CodeIgniter (if you are new to programming in general then CodeIgniter will be the easiest).

Next, why is your POST data JSON encoded? Why not just POST all the form variables? I would recommend that way instead to simplify it (even from JS).

Finally, you have to make sure you sanitize your inputs. You can use mysqli_real_escape_string(). I am assuming you will use the MySQLi DB interface. (Ref: http://php.net/manual/en/mysqlinfo.api.choosing.php)

Actually one last note: Laravel is, in my opinion, the future of PHP frameworks. It is beautiful, lightweight, and powerful. I HIGHLY recommend that you learn it. Ref: http://laravel.com/

Patrick
  • 3,302
  • 4
  • 28
  • 47
  • 1. A ORM framework won't be an advantage. If I need to create tables at run-time, as far as I know, an ORM would be a problem. 2.POST data is JSON encoded because I'm using Knockout.js. 3. I will sanitize it later. For now I just want to get it working. Afterwards, I could add some validation and protection. – Razvan Dec 12 '13 at 19:18
  • @razvan If you need to create variable schemas at runtime, you should perhaps be limiting those schemas to temp tables or possibly you should even be investigating NoSQL solutions that don't limit you to a specific schema. There is not problem with JSON data though I would say if you are going to POST JSON, actually POST JSON using `application/json` content type and decode the posted data from raw input rather than use a mix of JSON data sent within a form-encoded envelope (i.e. such that `$_POST` is populated). – Mike Brant Dec 12 '13 at 19:56
0

I managed to get it working. Now I'm constructing my queries based on the difference between the existing data and the updates from the user. The foreach loop now looks like this:

foreach ($existingData as $key => $value) {
    if ($existingData[$key] != $data[$key]) {
        $statement .=  $key . " = \"" . $data[$key] . "\" , ";
        $flag = true;
    }
}

This is the part that was interesting for me. The rest of the code should be updated according to the latest API.

Razvan
  • 3,017
  • 1
  • 26
  • 35