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

Dear perl monks, I have a questions which might be easy, but I am really stuck. I have the following script"

#!/usr/bin/perl use DBI; use Data::Dumper; use strict; use warnings; use CGI; my $fields; my $q= CGI->new(); # Connect to the database my $dbh = DBI->connect('DBI:mysql:mirnas', 'root', 'pass') or die "Couldn't open database: $DBI::errstr; stopped"; my $filename = $ARGV[0]; my $table_name = $ARGV[1]; open(INPUT, $filename); #getting the column names from the text file my $line = <INPUT>; chomp($line); my @fields = split('\t', $line); foreach $fields(@fields){ print "$fields\n"; } $sth = $dbh->prepare("create table $tablename ); $sth->execute;

What I want to do is to create a table where all the columnn names will be the ones stored in fields. I was thinking of using a loop structure where foreach $field it would add a column, but I am not sure if this is thebest way to do that. any suggestions would be really usefull, thank you in advance

Replies are listed 'Best First'.
Re: DBI question
by kennethk (Abbot) on Jun 22, 2010 at 15:01 UTC

    The presence of a use CGI; at the head of your script gives me pause as does using an unfiltered filename in a two-argument open and unfiltered, unescaped strings in your SQL. These are potentially very dangerous if exposed to the outside world, allowing execution of arbitrary code and SQL injection. These are a cracker's dream and an admin's nightmare. This is very nearly the worst you can do without malicious intent.

    Security aside, you can accomplish this with an algorithm that is essentially what you had in mind, though I would modify it a bit. Something like (untested):

    #!/usr/bin/perl use DBI; use strict; use warnings; # Connect to the database my $dbh = DBI->connect('DBI:mysql:mirnas', 'root', 'bi0u90ee') or die "Couldn't open database: $DBI::errstr; stopped"; my $filename = $ARGV[0]; my $table_name = $ARGV[1]; open(INPUT, '<', $filename) or die "Open failed $filename: $!"; #getting the column names from the text file my $line = <INPUT>; chomp($line); my @fields = split('\t+', $line); # prevent undefs foreach my $field (@fields){ print "$field\n"; } # SQL fragment for creating right number of columns my $column_fragment = <<EOSQL; ? TEXT NOT NULL, EOSQL $column_fragment x= @fields; $column_fragment =~ s/,\s*$/\n/; # Strip trailing comma # Whole CREATE TABLE statement with placeholders my $sql = <<EOSQL; CREATE TABLE ? ( $column_fragment ) EOSQL # Binding and execution my $sth = $dbh->prepare($sql) or die "Prepare failed: $DBI::errstr"; $sth->bind_param(1, $table_name) or die "Bind failed: $DBI::errstr"; for my $i (0 .. $#fields) { $sth->bind_param($i+2, $fields[$i]) or die "Bind failed: $DBI::err +str"; } $sth->execute or die "Execute failed: $DBI::errstr";

    Note that I've added success tests to every DBI call, added a test on the open, changed to a three-argument open to reduce exposure, and am using bound parameters in place of direct interpolation to handle character escaping. I'd also recommend editing your original post to scrub that password. See Placeholders and Bind Values in DBI.

Re: DBI question
by Tux (Canon) on Jun 22, 2010 at 15:07 UTC

    Please edit your post and remove the root password. Then change it on your host. (Unless you already posted a random root password in this post).

    Second, your code is quite unsafe. The filename can contain anything, and you don't check

    if (open my $fh, "<", $filename) { chomp (my $line = <$fh>); "$line\t" =~ m/^(?:[A-Za-z]\w*\t)+\z/ or die "Bad header in $filen +ame"; $dbh->do ("create table $tablename (". (join "," => map { "$_ varchar (20)" } split m/\t/ => $line)." +)"); }

    Would be a way to do what you ask, but you didn't e.g. define what types the fields have to be.


    Enjoy, Have FUN! H.Merijn
      Thank you both for the answers and suggestons. The problem lies now that the first filed is varchar (txt) but the rest are integers. So I am trying to do it first by creating the table and then adding the additional columns. any suggestions on that? Thanks again
        if (open my $fh, "<", $filename) { chomp (my $line = <$fh>); "$line\t" =~ m/^(?:[A-Za-z]\w*\t)+\z/ or die "Bad header in $filen +ame"; my ($fld1, @rest) = split m/\t/ => $line; $dbh->do ("create table $tablename (". (join "," => "$fls1 text", map { "$_ integer" } @rest).")"); }

        Of course TIMTOWTDI


        Enjoy, Have FUN! H.Merijn
        While you could either take the lump approach I've taken above or execute a single CREATE TABLE command followed by a series of ALTER TABLE ... ADD statements is fairly irrelevant from an algorithmic perspective. Both require that you specify the data type at run time. I would suggest using a hash to specify your desired types, and interpolate these into the appropriate SQL statements. Another possibility is a simple if-else clause.

        It sounds like you have a specific file and set of column names in mind - if this is the case, wouldn't it be easier to just craft the SQL rather than trying to do all this autodiscovery?