Skip to content

add ability to set generic PDO attributes#1904

Merged
dereuromark merged 5 commits intocakephp:masterfrom
MasterOdin:pdo_attr
Oct 19, 2020
Merged

add ability to set generic PDO attributes#1904
dereuromark merged 5 commits intocakephp:masterfrom
MasterOdin:pdo_attr

Conversation

@MasterOdin
Copy link
Copy Markdown
Member

Closes #1844

This allows specifying a list of PDO options that are then set against the connection using the setAttribute method. This follows a similar design of the adapter specific options of checking against a specific prefix. This is BC breaking in that it reserves attr_ for options, but there is no option with that value so it should not affect anyone's code unless they were adding extraneous things to their config.

I've also added documentation on setting these options, both the generic ones and the adapter specific ones with a simple example, as I could not find a mention of it before in the docs.

@garas
Copy link
Copy Markdown
Member

garas commented Oct 2, 2020

Array with real PDO constant keys would be better than error prone string keys.

"attributes" => [
    \PDO::ATTR_CASE => \PDO::CASE_LOWER,
    \PDO::MYSQL_ATTR_IGNORE_SPACE => 1,
],

@MasterOdin
Copy link
Copy Markdown
Member Author

I disagree with that. Going that route with using the constant names for keys would lock out JSON and YAML configuration files from using this part of phinx easily as you would be forced to use magic number keys which to me is way worse than strings. For your example, this would be the equivalent JSON object:

{
    "8": 1,
    "1009": 1
}

Additionally, it would probably mean that phinx would have to hardcode in the list of attributes for specific drivers so that you don't send \PDO::MYSQL_ATTR_IGNORE_SPACE to postgresql driver for example, and the docs say this about avoiding that scenario:

Using driver-specific attributes with another driver may result in unexpected behaviour.

@garas
Copy link
Copy Markdown
Member

garas commented Oct 2, 2020

I agree now, I was not familiar enough with how adapter specific attributes are handled.

@dereuromark dereuromark requested a review from othercorey October 2, 2020 16:35
Comment thread src/Phinx/Db/Adapter/PdoAdapter.php Outdated
Comment thread src/Phinx/Db/Adapter/PdoAdapter.php Outdated
Comment thread docs/en/configuration.rst Outdated
MasterOdin and others added 4 commits October 16, 2020 21:56
Co-authored-by: Mark Sch. <dereuromark@users.noreply.github.com>
Co-authored-by: othercorey <corey.taylor.fl@gmail.com>
@dereuromark
Copy link
Copy Markdown
Member

Looks good.

@dereuromark dereuromark merged commit 0cca945 into cakephp:master Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Be able to specify PDO::ATTR_ options for connection

4 participants