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

\PDO:: class constant aliases (request) #13010

Closed
cottton opened this issue Aug 6, 2017 · 3 comments
Closed

\PDO:: class constant aliases (request) #13010

cottton opened this issue Aug 6, 2017 · 3 comments
Milestone

Comments

@cottton
Copy link
Contributor

cottton commented Aug 6, 2017

By merging multiple service configs like

 return [
     'database' => [
         'adapter'    => '...',
         // ...
         'persistent' => '...',
         'options'    => [
             \PDO::MYSQL_ATTR_INIT_COMMAND => "...",
         ],
     ]
 ];

we got a problem with numeric keys (integer).
F.e. \PDO::MYSQL_ATTR_INIT_COMMAND === 1002.
So we get a actual config of:

 return [
     'database' => [
         'adapter'    => '...',
         // ...
         'persistent' => '...',
         'options'    => [
             1002 => "...",
         ],
     ]
 ];

In PHP if merging multiple configs we get renumbered keys ofc.
Example:

 // config 1
 return [
     'database' => [
         'adapter' => '...',
         // ...
         'options' => [
             \PDO::MYSQL_ATTR_INIT_COMMAND => "...",
         ],
     ]
 ];

 // config 2
 return [
     'database' => [
         'adapter' => '...',
         // ...
         'options' => [
             \PDO::MYSQL_ATTR_INIT_COMMAND => "...",
         ],
     ]
 ];

 // merged result
 return [
     'database' => [
         'adapter' => '...',
         // ...
         'options' => [
             0 => "...", 
             1 => "...",
         ],
     ]
 ];

The fatal here is that the key 0 will end up as a "AUTOCOMMIT=0".

To tmp fix that we can force keys to strings like

 'options' => [
     (string)\PDO::MYSQL_ATTR_INIT_COMMAND => "...",
],

To fix this perm i suggest allowing aliasses.
mysql_attr_init_command for \PDO::MYSQL_ATTR_INIT_COMMAND.

Im not into zephir. I cannot provide a PR. But i can provide the code in PHP.
Would be nice if somebody could convert that into zep for me.

Where to put:

https://github.com/phalcon/cphalcon/blob/master/phalcon/db/adapter/pdo.zep#L130

What to put:

// check for \PDP::XXX class constant aliasses
foreach ($options as $key => $value) {
    if (is_string($key) && defined(\PDO::class . "::" . strtoupper($key))) {
        $options[constant(\PDO::class . "::" . strtoupper($key))] = $value;
        unset($options[$key]);
    }
}

Thanks,
cottton

@stamster
Copy link
Contributor

stamster commented Aug 7, 2017

Well, this is from Phalcon's config object and toArray() method:

Array
(
    [host] => db.lxc
    [username] => myUser
    [password] => myPass
    [dbname] => myDb
    [charset] => UTF8
    [persistent] => 1
    [options] => Array
        (
            [19] => 5
        )

)

//19 => 5  originally was set as: \PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_OBJ

and vanilla PHP:

$m = [
\PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_OBJ,
\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
\PDO::ATTR_EMULATE_PREPARES => 0,
\PDO::MYSQL_ATTR_FOUND_ROWS => 1
];

print_r($m);

Array
(
    [19] => 5
    [3] => 2
    [20] => 0
    [1005] => 1
)


So those constants will always be converted internally to integer values, since that's what they are - you're using constant name in order to get it's value.

@cottton
Copy link
Contributor Author

cottton commented Aug 7, 2017

I know. Thats why im asking to add the alias feature.
in your example i would like to provide a config:

$m = [
    'attr_default_fetch_mode' => \PDO::FETCH_OBJ,
    'attr_errmode'            => \PDO::ERRMODE_EXCEPTION,
    'attr_emulate_prepares'   => 0,
    'mysql_attr_found_rows'   => 1
];

Test

// simulated options
$options = [
    'attr_default_fetch_mode' => \PDO::FETCH_OBJ,
    'attr_errmode'            => \PDO::ERRMODE_EXCEPTION,
    'attr_emulate_prepares'   => 0,
    'mysql_attr_found_rows'   => 1
];


// check for \PDP::XXX class constant aliasses
foreach ($options as $key => $value) {
    if (is_string($key) && defined(\PDO::class . "::" . strtoupper($key))) {
        $options[constant(\PDO::class . "::" . strtoupper($key))] = $value;
        unset($options[$key]);
    }
}


echo var_export($options);
// array (
//     19 => 5,
//     3 => 2,
//     20 => 0,
//     1005 => 1,
// )

Works fine. I just dont have enough knowledge about zep to provide a PR.

@cottton cottton mentioned this issue Sep 21, 2017
3 tasks
@sergeyklay sergeyklay added this to the 3.3.x milestone Nov 23, 2017
sergeyklay pushed a commit that referenced this issue Nov 23, 2017
* Allow PDO option aliases

See #13010

* fixing syntax error
@sergeyklay
Copy link
Contributor

Implemented in the 3.3.x branch. Thank you for contributing.

chilimatic pushed a commit to chilimatic/cphalcon that referenced this issue Jan 15, 2018
* Allow PDO option aliases

See phalcon#13010

* fixing syntax error
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

3 participants