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

add ability to set generic PDO attributes #1904

Merged
merged 5 commits into from
Oct 19, 2020
Merged

add ability to set generic PDO attributes #1904

merged 5 commits into from
Oct 19, 2020

Conversation

MasterOdin
Copy link
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
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
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
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
@dereuromark
Copy link
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