Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFR] Implement PDO::FETCH_GROUP in Phalcon\Db #1642

Closed
le51 opened this issue Dec 5, 2013 · 12 comments
Closed

[NFR] Implement PDO::FETCH_GROUP in Phalcon\Db #1642

le51 opened this issue Dec 5, 2013 · 12 comments

Comments

@le51
Copy link

le51 commented Dec 5, 2013

This feature allows you to get associated formatted arrays in a really simple manner.
Some exemples here:
http://forum.phpfrance.com/vos-contributions/affichage-par-categorie-avec-pdo-t254289.html
It's in french, but code is clear.

@dreamsxin
Copy link
Contributor

Like this:

$result->setFetchMode(Phalcon\Db::FETCH_COLUMN | Phalcon\Db::FETCH_GROUP);
$result->setFetchMode(Phalcon\Db::FETCH_ASSOC | Phalcon\Db::FETCH_GROUP);

@ghost
Copy link

ghost commented Dec 12, 2013

@le51 this has been implemented in both 1.2.5 and 1.3.0, could you please check?

@le51
Copy link
Author

le51 commented Dec 12, 2013

Hi, thank you for reactivity ... but I've just downloaded last 1.3.0 with:

git clone http://github.com/phalcon/cphalcon
cd build
git checkout 1.3.0
sudo ./install

and your PR aren't in there. Am I missing something ?

@ghost
Copy link

ghost commented Dec 12, 2013

Please build from ext/:

cd ext
git checkout 1.3.0
phpize
./configure
make
sudo make install

@le51
Copy link
Author

le51 commented Dec 12, 2013

Oh Ok, It's been a while since I was asking myself on how to build the different branch of phalcon. I've learned something helpfull today !!! Thank you

So, with Phalcon\Db::FETCH_GROUP, I've tested it:

$cities_pilots = $this->db->fetchAll("SELECT p.ville, p.nom, p.prenom FROM personne p WHERE p.ville LIKE 'A%' ORDER BY p.ville ASC", Phalcon\Db::FETCH_NUM|Phalcon\Db::FETCH_GROUP);
var_dump($cities_pilots);

But it doesn't output the expected array (grouped by "ville" name as the given link above said):
What I get:

Array()
...
12 => 
    array (size=3)
      'ville' => string 'AG NUENEN' (length=9)
      'nom' => string 'DADMEND (length=5)
      'prenom' => string 'PHILIPPE' (length=8)
  13 => 
    array (size=3)
      'ville' => string 'AH ALPHEN A/D RIJN' (length=18)
      'nom' => string 'KONINGINS' (length=7)
      'prenom' => string 'MISCHA' (length=6)
  14 => 
    array (size=3)
      'ville' => string 'AIX EN PROVENCE' (length=15)
      'nom' => string 'KAP' (length=8)
      'prenom' => string 'NICK' (length=7)
  15 => 
    array (size=3)
      'ville' => string 'AIX EN PROVENCE' (length=15)
      'nom' => string 'MANTIS' (length=6)
      'prenom' => string 'VIANNEY' (length=7)
  16 => 
    array (size=3)
      'ville' => string 'AIX EN PROVENCE' (length=15)
      'nom' => string 'MARCZAK' (length=7)
      'prenom' => string 'JEAN' (length=9)
....

By the way, I've tested the raw example:

<?php
try
{
    $pdo = new PDO('pgsql:host=localhost;dbname=myDb', 'myUserName','myPassword');
    $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
}
catch (Exception $e)
{
    exit($e->getMessage());
}

$query = $pdo->query("SELECT u.ville, u.nom, u.prenom FROM personne u WHERE u.ville LIKE 'A%' ORDER BY u.ville, u.nom ASC");

if($query)
{
    $result = $query->fetchAll(PDO::FETCH_ASSOC | PDO::FETCH_GROUP);
    var_dump($result);
}
?>

And result is as expected:

Array()
....
  'AH ALPHEN A/D RIJN' => 
    array (size=1)
      0 => 
        array (size=2)
          'nom' => string 'KONINGINS' (length=7)
          'prenom' => string 'MISCHA' (length=6)
  'AIX EN PROVENCE' => 
    array (size=46)
      0 => 
        array (size=2)
          'nom' => string 'ABOU' (length=7)
          'prenom' => string 'AMANDA' (length=8)
      1 => 
        array (size=2)
          'nom' => string 'BARRET' (length=6)
          'prenom' => string 'MARC (length=8)
      2 => 
        array (size=2)
          'nom' => string 'BRCK' (length=6)
          'prenom' => string 'PIERRE' (length=11)
      3 => 
....

(I've modify the names to keep users "anonymous, so lengths are not the good one !)

@ghost
Copy link

ghost commented Dec 13, 2013

OK looking

@ghost
Copy link

ghost commented Dec 13, 2013

Looks like the issue is how Phalcon implements fetchAll() internally: it calls $stmt->fetch() in the loop.

I have updated the implementation to use $stmt->fetchAll() instead:

diff --git a/ext/db/adapter.c b/ext/db/adapter.c
index 1efb5a2..924e76b 100644
--- a/ext/db/adapter.c
+++ b/ext/db/adapter.c
@@ -264,8 +264,7 @@ PHP_METHOD(Phalcon_Db_Adapter, fetchOne){
 PHP_METHOD(Phalcon_Db_Adapter, fetchAll){

        zval *sql_query, *fetch_mode = NULL, *bind_params = NULL, *bind_types = NULL;
-       zval *result, *row = NULL;
-       zval *r0 = NULL;
+       zval *result;

        PHALCON_MM_GROW();

@@ -284,26 +283,14 @@ PHP_METHOD(Phalcon_Db_Adapter, fetchAll){
                bind_types = PHALCON_GLOBAL(z_null);
        }

-       array_init(return_value);
-
        PHALCON_INIT_VAR(result);
        phalcon_call_method_p3(result, this_ptr, "query", sql_query, bind_params, bind_types);
        if (likely(Z_TYPE_P(result) == IS_OBJECT)) {
                if (Z_TYPE_P(fetch_mode) != IS_NULL) {
                        phalcon_call_method_p1_noret(result, "setfetchmode", fetch_mode);
                }
-
-               while (1) {
-
-                       PHALCON_INIT_NVAR(r0);
-                       phalcon_call_method(r0, result, "fetch");
-                       PHALCON_CPY_WRT(row, r0);
-                       if (!zend_is_true(row)) {
-                               break;
-                       }
-
-                       phalcon_array_append(&return_value, row, 0);
-               }
+
+               phalcon_return_call_method_p0(result, "fetchall");
        }

        PHALCON_MM_RESTORE();

@le51
Copy link
Author

le51 commented Dec 14, 2013

Hi, sjinks

I've tried your fix, but I'm still getting the same result (as above) ...

---- OFF TOPIC ----
How can I apply a patch, like yours, wich has not been merged ? (I did the compilation after having replaced phalcon's adapter.c source file with that one:
https://github.com/sjinks/cphalcon/blob/af105b8656ab9b9a9fcf54f2fb957388382630ce/ext/db/adapter.c)

phalcon pushed a commit that referenced this issue Dec 15, 2013
#1642: use fetchAll() instead of fetch() in Phalcon\Db\Adapter::fetchAll()
@ghost
Copy link

ghost commented Dec 22, 2013

Phalcon's implementation corresponds to this:

$dbh    = new PDO('mysql:host=localhost;dbname=phalcon_test', 'user', 'pass');
$query  = $dbh->query("SELECT email, nombres FROM personas");
$query->setFetchMode(PDO::FETCH_ASSOC | PDO::FETCH_GROUP);
$result = $query->fetchAll();
print_r($result);

That is, fetch options are set by setFetchMode(), not as arguments to fetchAll() and PDO does not seem to like it.

I'll check what can be done here.

@temuri416
Copy link
Contributor

Got a bit confused by the commits around Phalcon\Db::FETCH_GROUP work.

Is grouping currently supported by Phalcon or is it still under development?

Thanks!

@le51
Copy link
Author

le51 commented Dec 28, 2013

Hi,
sorry, I'am on in vacations far away from my dev computer so I can't test the latest commits.

Have a nice day.

@ghost
Copy link

ghost commented Dec 30, 2013

Under development

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants