Building queries with PDO?

Question

I have some ancient code which I want to convert to PDO:

<?php
    function build_query() {
        // db connection here

        $the_query = "";

        if ( empty( $_GET['c'] ) ) {
            $the_query = "select * from table1";

            if ( ( isset( $_GET['y'] ) ) && ( isset( $_GET['m'] ) ) ) {
                $the_query .= " where y = " . $_GET['y'] . " and m = " .  $_GET['m'];
            }
        } elseif ( ( $_GET['c'] == "1" ) || ( $_GET['c'] == "2" ) ) {
            $the_query = "select * from table1 where GGG = " . $_GET['c'];

            if ( ( isset( $_GET['y'] ) ) && ( isset( $_GET['m'] ) ) ) {
                $the_query .= " and y = " . $_GET['y'] . " and m = " .  $_GET['m'];
            }
        } else {
            $the_query = "select * from table1";

            if ( ( isset( $_GET['y'] ) ) && ( isset( $_GET['m'] ) ) ) {
                $the_query .= " where y = " . $_GET['y'] . " and m = " .  $_GET['m'];
            }

            $the_query .= " and c = " . $_GET['c'];
        }

        return // use the query to return results $the_data;
    }
?>

I can't seem to figure out how to recode this using PDO. I have made a start below, but can't seem to get any further:

<?php
    function build_query() {
        $the_data = "";

        $DBH = new PDO( "mysql:host=server;dbname=database", "user", "pass" );
        $DBH -> setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );

        $STH = $DBH -> prepare( "build query here" );

        $STH -> bindParam( ':c', $_GET['c'], PDO::PARAM_INT );
        $STH -> bindParam( ':y', $_GET['y'], PDO::PARAM_INT );
        $STH -> bindParam( ':m', $_GET['m'], PDO::PARAM_INT );

        $STH -> execute();

        $ROWS = $STH -> fetchAll();

            foreach($ROWS as $ROW) {
            $output .= $ROW["a"] . " - " . $ROW["b"] . " - " . $ROW["c"] . " - " . $ROW["d"] . "<br />";
            }

        $DBH = null;

        return $output; 
    }       
?>

are you really connecting to database every time you run a query? — Your Common Sense, Sep 30 '11 at 14:40
but why? any reason for this? just because it was in the tutorial? — Your Common Sense, Sep 30 '11 at 14:42
@stereofrog, If I do continue to build the query, how do I set the bindParam's to only be used when needed depending on the conditions used to create the query? For example, if the first `if` is used, I don't need the `c` parameter. — oshirowanen, Sep 30 '11 at 14:45
@stereofrog, PDO does help, it gets right of the unreadable mess that is the above code. And it fixes the SQL-injection holes in version 1. — Johan, Sep 30 '11 at 15:31
@stereofrog, see: http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain — Johan, Oct 04 '11 at 11:46
@Johan, What I mean is, how do I convert that mess in my original code to pure PDO? — oshirowanen, Oct 05 '11 at 11:11
@oshirowanen, you cannot use `pure` PDO, PDO does not support dynamic SQL see col. Schrapnel's answer on how to do this safely. — Johan, Oct 05 '11 at 11:22

score 2 · Accepted Answer · answered Sep 30 '11 at 14:56

Well, it's quite tricky with prepared statements
(that's why I prefer my home-brewed placeholders over prepared statements)

First of all you have to make this ancient code sensible, without all that current mess. Check every parameter only once.

here is a code to give you an idea

$w = array();
if ( !empty($_GET['c']) AND ($_GET['c'] == "1" ) || ( $_GET['c'] == "2") )
{
    $w[] = $db->parse("GGG = ?i", $_GET['c']);
}
if ( isset($_GET['y']) && isset($_GET['m']) )
{
    $w[] = $db->parse("where y = ?i and m = ?i",$_GET['y'],$_GET['m']);
}
$where = '';
if ($w) $where = implode(' AND ',$w);
$query = "select * from table1 $where";

to make use of prepared statements you have to add your values into array and then use it with execute()

+1 You wouldn't happen to have a link to those home-brewed placeholders would you? — Johan, Sep 30 '11 at 15:32
Well, it become available just recently. https://github.com/colshrapnel/safemysql if you're still interesting. You're welcome to use and criticize it :) — Your Common Sense, Mar 05 '13 at 14:34

jeroen · Answer 2 · 2011-09-30T15:00:43.883

0

You are changing your function to do a lot more that it used to do. If you want to stick to the original design (that is consistent with the name of your function...), you need to change your function so that it still returns a query but does not execute it as the old version did not connect to a database or query a database either.

To solve your problem, you can have the function return an array with 2 elements, the query with the named parameters and another array with the name - value pairs.

If you do want to return the results of the query, you can use a global variable for your database connection or pass it as a variable to the function.

edited Sep 30 '11 at 15:00

answered Sep 30 '11 at 14:54

jeroen

91,079
21
114
132

You 're way overcautious :) `PDO::PARAM_INT does not really cast your variable to int` - what does it do then? – Your Common Sense Sep 30 '11 at 15:03
@Col. Shrapnel If I recall correctly, nothing, but I can't find a reference so I've removed it... – jeroen Sep 30 '11 at 15:04
I'm curious, if I build a query using a similar function as my original code where it does not execute the query, i.e. it just builds it and returns it when the build function is called, so nothing to prevent sql injections, and then I simply return the string into PDO like so: `$STH = $DBH -> prepare( $build_query_string );` Will that prevent SQL Injections? – oshirowanen Sep 30 '11 at 15:08
@oshirowanen how do you think? – Your Common Sense Sep 30 '11 at 15:09
@Col. Shrapnel, I'm guessing it will not protect against sql injections, but I am not 100% sure as I don't know pdo enough to be able to say yes or no with 100% certainty? – oshirowanen Sep 30 '11 at 15:11

Building queries with PDO?

2 Answers2