kiat has asked for the wisdom of the Perl Monks concerning the following question:

Hey Monks,

I need your advice and feedback on the following (partial) code. What it does is removing checked values of an html form from a mysql table. Each value is named with an identical value 'id'
# sample source from html <form name="samp_db"> Row 1: Megan, Column 2: F, Column 3: 1<input type="checkbox" name="id" + value="1"><br> Row 2: Joseph, Column 2: M, Column 3: 2<input type="checkbox" name="id +" value="2"><br> Row 3: Kyle, Column 2: M, Column 3: 3<input type="checkbox" name="id" +value="3"><br> Row 4: Katie, Column 2: F, Column 3: 4<input type="checkbox" name="id" + value="4"><br> Row 5: Abby, Column 2: F, Column 3: 5<input type="checkbox" name="id" +value="5"><br> <input type="submit" name="submit" value="delete"> </form> # perl code to delete checked values my $dbh = DBI->connect("DBI:${dbtype}:${dbname}", "$dbuser", "$dbpass" +) || die $DBI::errstr; my ($sql, $sth) = ''; # Remove checked values # This is the part of the code that I'm seeking advice on... foreach ($cgi->param('id')) { # name of table is student $sql = "DELETE FROM student WHERE (student_id = $_)"; $sth = $dbh->prepare($sql); $sth->execute() || die $DBI::errstr; } # prints updated table $sql = "SELECT * FROM student"; $sth = $dbh->prepare($sql); $sth->execute() || die $DBI::errstr; # show() is a subroutine that outputs the updated table in html format show();
Is that the way to accomplish deletion of entries in a table from a mysql database?

Replies are listed 'Best First'.
Re: Delete multiple values from mysql db..
by gellyfish (Monsignor) on Jun 16, 2003 at 10:04 UTC

    Skeeve's advice on using placeholders is very sensible. However it is not simply a matter of efficiency or elegance but one of security. As your code stands right now it would be a simple matter for some bad person to give the query parameter 'id' a value such as:

    1) ; (drop table student
    and your day would begin to take a turn for the worse.

    Really you should be checking the values of 'id' you are using for validity and skipping any which are not purely digits. You probably also want to use the -T switch to Perl and DBI's 'Taint' option to cause DBI to croak if it is passed tainted data.

    You might have something like this replacing your foreach loop:

    my $sql = 'DELETE FROM student where student_id = ?'; my $sth = $dbh->prepare($sql); foreach my $id ( $cgi->param('id') ) { if ( $id =~ /^(\d+)$/ ) { $id = $1; $sth->execute($id); } }

    /J\
    

Re: Delete multiple values from mysql db..
by Skeeve (Parson) on Jun 16, 2003 at 08:43 UTC
    I'm relatively new to DBI, but it seems okay to me.

    You might want to have a look at the database part oft the Tutorials to find a better way to achieve your deletion. (hint: placeholders)

Re: Delete multiple values from mysql db..
by sgifford (Prior) on Jun 16, 2003 at 17:05 UTC
    You can do it with one query (and with better security) by replacing your foreach loop on the id param with something like:
    $sql = "DELETE FROM student WHERE student_id IN (". join(",",map {$dbh->quote($_)} $cgi->param('id')). ")"; $sth = $dbh->prepare($sql); $sth->execute() || die $DBI::errstr;
    You really should be running CGI scripts in taint mode (-T); it will help you catch common but serious errors like forgetting to quote input from forms.
Re: Delete multiple values from mysql db..
by Itatsumaki (Friar) on Jun 16, 2003 at 15:28 UTC

    Incidentally, you may want to turn off auto-commit behaviour and throw a $dbh->commit() after your foreach loop. That way, if one delete fails prior ones are not executed. Just a thought.

    -Tats
Re: Delete multiple values from mysql db..
by Anonymous Monk on Jun 16, 2003 at 09:48 UTC
    ofcourse you can delete the checked items from the database and MySql connection is also fine. But problem is in your checked box, you have to identify each item... For doing this, in the checkbox append value to the name field (ie name="id1",name="id2" etc). So in your perl code , you can catch id values into the array and delete accordingly. Hope this may help. Sureshp

      No that bit of it is fine. Using the CGI module if a single parameter name has multiple values then the param() method will return a list of the values instead of a scalar.

      /J\