Help with PDO class

Hi there
I’m trying to convert an old procedural database handler class to php’s native PDO
While I’m at it, I’m also trying to add support for slave and master servers in SQL

However, I can’t seem to get the main query function to return what I want…here’s the code:



function slave_query($unsafequery) {
		//this returns a PDO connection
		$conn=$this->get_slave_connection();
		$safequery=$conn->quote($unsafequery);
		$result=$conn->prepare($safequery);
		$result->execute();
		return $result;
	}

I am expecting the function to return a $result variable that I can loop through using “$result->fetch()” but instead a var_dump on $result gives me this:


object(PDOStatement)#7 (1) { ["queryString"]=> string(36) "'SELECT email FROM members LIMIT 10'" } 

Ideally, in my front-end page, I’d like to do this:


$result=$db->slave_query("SELECT email FROM members LIMIT 10");
while ($result->fetch()) {
//display data
}

Could anybody help? It’s the first time I use PDO and don’t quite know how to use it :confused:

You are returning the value of $result. Hence, you are returned the prepare statement, so the dump is indeed what we should expect.
The execute() does what it says, it executes the query, so you will receive either true or false after that.

According to this, it seems that, returning $result on that stage will be of no use for you.

So, you must, instead of:


$result=$conn->prepare($safequery);
$result->execute();
return $result;

Make:


$result = $conn->prepare($safequery);
$result->execute();
$data = $result->fetch(PDO::FETCH_ASSOC);

return $data;

And then you will use $data on your front-end.
And if you use fetchAll() instead of fetch, and you will receive an array of values, if you may got those as well from your query.

Does this make sense? Or am I missing the point?

Regards,
Márcio

I tried doing as you suggested but all I get, even with valid queries, is just a FALSE :confused:

Does it have something to do with what they mention here? -> PHP: PDOStatement->fetch - Manual

The line

$safequery=$conn->quote($unsafequery);

doesn’t make any sense. quote is for quoting individual string values that become part of a longer sql statement, not for quoting the whole thing.

And that may also be the reason for the returning false that you are having.

I would say: remove as much variables that you have there, keep it simple and test it.

I would also suggest to remove/replace

$safequery

with a direct query instruction

"SELECT email FROM members LIMIT 10"

And see what you get.

I would say, try something among this lines:


/**
* @desc It retrieves an unsafe query, and returns the data:
*/
function slaveQuery($unsafequery) 
{
   //this returns a PDO connection
   $conn=$this->getSlaveConnection();
   
   //the prepare will deal with your unsafequery:
   $result=$conn->prepare($unsafequery);

   $result->execute();

   //not sure if you want to return always only one row, or several, I tend to use fetchAll instead of just fetch.
   $data = $result->fetchAll(PDO::FETCH_ASSOC);

   return $data; 

} 

Regards,
Márcio

So it works kind of like the old mysql_real_escape_string ?

I’m wondering…why have a slave_query or slave_* anything? Why not just have two database objects?

Example:


$master = new PDO( '...' );
$slave   = new PDO( '...' );

You were right, removing the “quote” part solved the “FALSE” problem
I’m now wondering though, what would be the most convenient way of auto-quoting the values in the queries?

Because on the front-end I’m calling this:


$sql=$db->slave_query("SELECT email FROM members LIMIT 10");

and on the backend I have this


function slave_query($unsafequery,$slave=FALSE) {
		//default slave in case it's not specified
		$conn=$this->get_slave_connection($slave);
		$result=$conn->prepare($unsafequery);
		$result->execute();
		$data = $result->fetchAll(PDO::FETCH_ASSOC);
		return $data;
	}//endof

Also, I seem to recall that mysql_real_escape_string actually uses a connection to the database to escape the string, so I cannot use that in this case (I’ll be working with several slaves and several masters, and I have no way of specifying which connection should be used, not to mention it’s a bad idea performance wise, I’d much prefer to do this locally and not over TCP)

On the other hand, I’d prefer not to rely on “remembering to escape manually every time I send a query” because it leaves a security risk (i.e. whenever I forget to escape we have a security blunder)

What is the most common way of escaping the queries in this case?

@logic_earth:
Having 2 separate sets of queries helps me make my code more readable, and I can implement an auto fail-over so if slave 1 fails I can redirect to slave 2, or to the master, all automatically

Well you could do that with separate objects by having a manager that would be the intermediator between them. I’m just saying is all.
I think it would just be smarter without making a bunch of duplicate code for master_query, slave_query, slave2_* etc.

I get your drift but I’m not quite sure how I would implement that? Your idea is interesting and I do have quite a bit of repetitive code that I’d like to get rid of :slight_smile:

Would you mind sharing a practical example please?

Well, it would probably take a bit more work and thought then what I currently have to provide. But it should help get the idea across despite being quick and dirty.

Use:


$master = new MasterDatabase( 'con...' );
$slave1 = new SlaveDatabase( 'con...' );
$slave2 = new SlaveDatabase( 'con...' );

$con = new DatabaseController( $master );
$con->addSlave( $slave1, $slave2 );

$con->query( 'query...' );


abstract class GenericDatabase
{
  public function __construct ( $con )
  {
    // connection...
  }
  
  public function query ( $query )
  {
    // query...
    // query failed! send exception.
    throw new DatabaseException( 'QUERY FAILED OMG!!!' );
  }
}

class MasterDatabase extends GenericDatabase {}
class SlaveDatabase extends GenericDatabase {}

#-------------------------------------------------------------------------------

class DatabaseController
{
  protected
    $master,
    $slaves  = array();
  
  public function __construct ( MasterDatabase $master )
  {
    $this->master = $master;
  }
  
  public function addSlave ( SlaveDatabase $slave )
  {
    // addSlave( $slave, $slave, $slave... )
    if ( func_num_args() > 1 ) {
      foreach ( func_get_args() as $arg ) {
        if ( $arg instanceof SlaveDatabase )
          $this->slaves[ spl_object_hash( $arg ) ] = $arg;
      }
      
      return $this;
    }
    
    $this->slaves[ spl_object_hash( $slave ) ] = $slave;
  }
  
  #-------------------------------------------------------------------------------
  
  public function query ( $query )
  {
    try {
      return $this->master->query( $query );
    } catch ( DatabaseException $e ) {
      foreach ( $this->slaves as $slave ) {
        try {
          return $slave->query( $query );
        } catch ( DatabaseException $e ) {
          continue;
        }
      }
      
      // IF every single db failed....
      throw $e;
    }
  }
}

Thanks for helping, now I get what you meant :slight_smile:

As far as I can tell, since you are using PDO, the prepare method IS the proper way. It will escape, and it will use the PDO::quote() as well.

The parameters to prepared statements don’t need to be quoted; the driver automatically handles this. If an application exclusively uses prepared statements, the developer can be sure that no SQL injection will occur (however, if other portions of the query are being built up with unescaped input, SQL injection is still possible).

source:PHP: Prepared statements and stored procedures - Manual

Sorry for the late reply,
Márcio

No escaping occurs when using prepared statements. The query is sent completely separate of the bind variables. Furthermore, when no bind variables exist a prepared statement is unnecessary but more importantly adds a useless trip to the database.

(: Thanks for correcting me. Worst then have a bad opinion, if to believe that that opinion is true.

I’ve seen that statement before (useless usage of prepare statements when no values are being passed), but I’ve seen so much examples around the web doing otherwise that I was start questioning me about some benefits that I may not be aware.

A refresh on php website, quicky vanish this bad decision, and shows no evidence of prepare statement usage without parameters on the query.

The reason why that is is because normally frameworks and reusable functions/methods are written to always except bind variables and only handle that case. In theory it is relatively easy to anticipate the possibility of no bind variables and handle the query without a prepared statement. Both the query and execute method return same object so its not that much logic to switch between prepared statements and unprepared.