Looking for neater solution with dynamic query

[pg]

I need to query database base on the search criteria coming with the URL. I had this code below, but one thing that I really don't like is the fact that I am not actully benefited from prepare and binding any more. Well, I am looking for something neater.

  
 if (defined($query{"component"}) || defined($query{"env"})) {
    my $dbh = DBI->connect("DBI:mysql:test", "root", "930612", {'Raise
+Error' => 1});
    my $query = "SELECT * FROM or_mod";
    if (defined($query{"component"})) {
     if ($query eq "SELECT * FROM or_mod") {
      $query = $query . " where upper(component) = upper('" . $query{"
+component"} . "')"; 
#I am forced to deal with quotes myself, as I am not binding later
     } else {
      $query = $query . " and upper(component) = upper('" . $query{"co
+mponent"} . "')"; 
# I know this is not needed here. I did it just in case some day someo
+ne reverse the order of codes
     }
    }
    if (defined($query{"env"})) {
     if ($query eq "SELECT * FROM or_mod") {
      $query = $query . " where upper(env) = upper('" . $query{"env"} 
+. "')";
     } else {
      $query = $query . " and upper(env) = upper('" . $query{"env"} . 
+"')";
     }
    }
    $query = $query . " order by env, application, component, mod_date
+ desc";
    print $query;
    my $sth = $dbh->prepare($query); 
#This prepare is unreal. One is supposed to prepare a statement only o
+nce
    $sth->execute();

[merlyn]

See SQL::Abstract for a perl-ish way to define things like this, and don't forget prepare_cached instead of prepare to try to reuse dynamically constructed SQL queries.

[pg]

Cool module!

Although the module does not resolve the binding issue, I think the database binding ability is lost any way with dynamic query. So that's fine. (Update: I was mistaken about this. See below davidrw's solution. His solution allows dynamic query to take full benefit of binding.)

Side Note: although that module supports "binding", that's not the real binding. From database point of view, the queries prepared in this way are submited as literal, which will run slower than queries with real database binding that happens after prepare.

With literal query, database sees it as a new query every time it is submited, so there is less optimization done. Where as non-literal queries, the optimization is done once, and used ever after, until the query is flahsed off the query pool, due to size limitation of the pool.

[CountZero]

A lot will depend on the way DBD for this type of database is implemented. Last time I looked at it, DBD:mysql did all the binding internally and presented a literal query string to the database-engine (the prepare-method wasn't even send to the database!), so there was no speed benefit in binding. It may have changed in more recent versions. You still had the security benefit and the cleaner code of course.

[davidrw]

If you are constructing the query yourself (as opposed to some of the above suggestions), you can still use placeholders. Here's a generic format (very similar to the SQL::Abstract approach):

my $sql = "SELECT ... FROM table WHERE 1=1 ";
my @bind;
if( $query{env} ){
  $sql .= " AND upper(env) = upper(?) ";
  push @bind, $query{env};
}
if( $query{component} ){
  $sql .= " AND upper(component) = upper(?) ";
  push @bind, $query{component};
}
...
my $sth = $dbh->prepare($query);
$sth->execute(@bind);
# or use one of the $dbh->selectXXXX($query, {}, @bind)  methods of DB
+I

[pg]

I really like this. To push those bindings into an array does the trick. In this way, I am taking the full benefit of database binding, and yet allow me to construct the query on fly.

I am going with this solution. Thank you very much!

Oh that 1=1 is neat.

[tlm]

Oh that 1=1 is neat.

TRUE should work too.

[jZed]

In addition to merlyn's suggestion, SQL::Interpolate and SQL::String offer alternate approaches to dynamically constructing SQL.

[sgifford]

Are you looking for performance benefits from using prepare and binding parameters, or security benefits, or code-cleanliness benefits, or?...

To get the security benefits, I'll often maintain two parallel structures: one with the query with parameters replaced by ?, and another with all of the parameters that should be used in execute. For example:

  $query .= ' where upper(env)=upper(?)';
  push(@sql_params,$query(env});
  ...

  my $sth = $dbh->prepare($query) or die;
  $sth->execute(@sql_params);

Getting the performance benefits isn't as easy, but there aren't performance benefits unless you're in a long-running environment (like mod_perl) and frequently repeat the same query. If that's the case, if you can come up with a few parameterized queries, you can prepare all of them, then just pick the right one. For example, it looks like there are only 8 possibilities in your sample code, so you could store these 8 queries in an array, then pick the right one and send it the right parameters.

Also, you can simplify the way you append to the query by using an array and join (untested, but you get the idea):

if (defined($query{component}))
  push(@querypart, 'where upper(component) = upper(?)';
  push(@sql_params,$query{component});
}
if (defined($query{env})) {
  push(@querypart, 'where upper(env) = upper(?)';
  push(@sql_params,$query{env});
}

$query = 'SELECT * FROM or_mod ' .
         join(' AND ',@queryparts) .
         ' order by env, application, component, mod_date, desc';

print $query;
my $sth = $dbh->prepare($query); 
$sth->execute(@sql_params);

[eric256]

Unless I missed something I think there are only 4 possiblities.

my $queries = (
"SELECT * FROM or_mod;"
"SELECT * FROM or_mod WHERE upper(component) = upper(?)";
"SELECT * FROM or_mod WHERE upper(env)       = upper(?)";
"SELECT * FROM or_mod WHERE upper(component) = upper(?) AND 
                           upper(env)       = upper(?)";
);
my $sth; my @bind;
if (defined($query{env}) &&(defined $query{component}) {
   $sth = $dbh->prepare($queries->[3]);
   $sth->execute($query{component}, $query{env});
} elsif ( defined ($query{env}) ) { 
   $sth = $dbh->prepare($queries->[2]);
   $sth->execute($query{env});
} elsif ( defined ($query{component}) ) {
   $sth = $dbh->prepare($queries->[1]);
   $sth->execute($query{component});
} else {
   $sth = $dbh->prepare($queries->[0]);
   $sth->execute;
}

I'm not entirely sure that is much better than dynamicaly generating them, or better at all for that matter. In fact I think the dynamic solution is better because it is dynamic. BTW newer MySQL caches all queries whether you ask or not, I don't know if that holds for bound queries as well. If it does then as long as you are binding, even if it seems lame for this single call, it will give you benefits in the long run.

[pg]

I had this thought... the problem is that this cannot be used as a generic solution, as one can easily run into queries that have more complex where conditions.

However there is one benefit, the queries are now real prepared/binding queries. It is a taken and given thing. I decided to pass.

[InfiniteSilence]

Well, at least this will make things a bit more readable for the sql string creation:

#!/usr/bin/perl -wT
use strict;
use CGI qw|:standard :debug|;

my $sql_that_i_dont_know = 'select foo from foo where donkeys="fly"';
 
my $connector = 'where'; #by default

if($sql_that_i_dont_know=~m/where/){$connector = 'and'};
for(qw|component env|){
  next unless CGI::param($_); 
  $sql_that_i_dont_know .= qq| $connector upper($_) | . q| = "| . uc(C
+GI::param($_)) . q|"|; 
  }

  print $sql_that_i_dont_know;
1;

[atcroft]

While I suspect this might not be that great from the database side, would it be possible to do a query that contained all of the possible (field like ?) statements, preparing that query, then setting the values to '%' if they are not presented? Something such as:

  my $query = qq{SELECT * FROM table_name WHERE (field1 LIKE ?) AND 
(field2 LIKE ?) AND ... AND (fieldn LIKE ?) ORDER BY ...};
  my $rv = $sth->prepare($query);
  my @query_for = ('%') x $number_of_fields;
  # then replace those elements in @query_for 
  # with values from whereever, where appropriate
  $sth->execute(@query_for);

Just wondering...

[pg]

My concern is: if the database is not smart enough, this could confuse it, and optimization/performance becomes a real issue. I think that this was also the concern you expressed ;-)