Extended PDO

[harikt]

Hi Paul,

I was looking into the fetchAll method of the extended PDO and the fetchAll method of the normal PDO.

/**
 * 
 * Fetches a sequential array of rows from the database; the rows
 * are represented as associative arrays.
 * 
 * @param string $statement The SQL statement to prepare and execute.
 * 
 * @param array $values Values to bind to the query.
 * 
 * @param callable $callable A callable to be applied to each of the rows
 * to be returned.
 * 
 * @return array
 * 
 */
public function fetchAll($statement, array $values = array(), $callable = null)
{
    $this->bindValues($values);
    $sth = $this->query($statement);
    $data = $sth->fetchAll(self::FETCH_ASSOC);
    if ($callable) {
        foreach ($data as $key => $row) {
            $data[$key] = call_user_func($callable, $row);
        }
    }
    return $data;
}

http://www.php.net/manual/en/pdostatement.fetchall.php

public array PDOStatement::fetchAll ([ int $fetch_style [, mixed $fetch_argument [, array $ctor_args = array() ]]] )

You can see in ExtendedPdo they will not get a chance to make the class bind to the return values. Will it be good, if we can make optional arguments as that of fetchAll method signature ?

http://php.net/manual/en/pdostatement.setfetchmode.php

This is with respect to my recent experiment on fetchMode.

<?php
require __DIR__ . '/vendor/autoload.php';

class PollChoice 
{
    protected $id;
    protected $poll_id;
    protected $choice_text;
    protected $votes;

    public function __get($key)
    {
        return $this->{$key};
    }

    public function __set($key, $value)
    {
        $this->{$key} = $value;
    }
}

use Aura\Sql\ExtendedPdo;
use Aura\Sql\Profiler;

$pdo = new ExtendedPdo(
    'mysql:host=localhost;dbname=dbname',
    'username',
    'password',
    array(), // driver options as key-value pairs
    array()  // attributes as key-value pairs
);

$sql = 'SELECT * FROM polls_choice WHERE id IN (:id) AND poll_id = :poll_id';

$bind = array(
    'id' => array('1', '2'),
    'poll_id' => '1',
);
$pdo->bindValues($bind);
$results = $pdo->query($sql, PDO::FETCH_CLASS, 'PollChoice');
foreach ($results as $result) {
    echo "Query Result : " . $result->choice_text . PHP_EOL;
}

Let me know your thoughts.

[pmjones]

It took me a while to understand what you were getting at. I think I understand now.

Having the fetchAll() (and perhaps other methods) take yet another (4th) argument makes me think we are trying to shove too much functionality into the individual methods. I don't want to find out later that we need a 5th, 6th, etc set of arguments.

One option is to make the 3rd param a generic specification, and the 4th param the target of the specification, just as with PDO now. For example, the existing functionality would be this (note the new FETCH_CLOSURE constant):

$pdo->fetchAll($stmt, $bind, $pdo::FETCH_CLOSURE, function () {
    // the closure to process results
});

And the functionality you are asking for would be this:

$pdo->fetchAll($stmt, $bind, $pdo::FETCH_CLASS, 'PollChoice');

That might increase the complexity of the various fetch methods, but I'm not opposed to doing so if needed.

Your thoughts?

[harikt]

@pmjones my question can't we set a method to set the fetch method. So the fetchAll will not use $data = $sth->fetchAll(self::FETCH_ASSOC);

But something like $data = $sth->fetchAll($this->getFetchMethod()); something like that ?

[harikt]

and your example is also a good one.

[pmjones]

You are confusing $sth (which is a PDOStatement object) with $pdo (which is a PDO or ExtendedPDO object). Be careful not to mix up your expectations of each one.

[harikt]

I thought we could have placed a setFetchMethod on the ExtendedPdo and if none is set we set FETCH_ASSOC as default for the fetchAll method. I understood it is on statement , but was just thinking.

[pmjones]

Let's not over-complicate things. If people want fine control over how things work, they can continue to work with PDOStatement objects as they have in the past. The point here is not to anticipate every possible need, but to make common cases easy.

[harikt]

ok :) .

Hari K T

http://harikt.com , https://github.com/auraphp

On Mon, Oct 7, 2013 at 1:54 PM, Paul M. Jones notifications@github.comwrote:

Let's not over-complicate things. If people want fine control over how
things work, they can continue to work with PDOStatement objects as they
have in the past. The point here is not to anticipate every possible need,
but to make common cases easy.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/59#issuecomment-25791740
.

[pmjones]

Hey now, the other part (about modifying fetchAll() to use the underlying FETCH_* modes and params) might still be useful. I'm reopening so I can keep it on my mind a little more. :-)

[pmjones]

After working this over a couple different ways, I'm going to leave the method signature the way it is. Thanks for the suggestion, and your patience.

[pmjones]

After a short talk with @feketegy I realized we can add just two methods, fetchOneObject() and fetchAllObject() to do exactly what @harikt is asking for. Reopening this until I implement it.

[harikt]

@pmjones you have all the rights to open and close :)

[harikt]

@pmjones can we close this for you have implemented it.

[pmjones]

Ah so! Thanks for the reminder.