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

Document the shape of the connection parameters array #4449

Merged
merged 7 commits into from
Nov 25, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Nov 25, 2020

Q A
Type improvement
BC Break no

This pull request documents the shape of the connection parameters array and addresses some issues discovered by the static analysis. It also enforces documentation of the newly added parameters (I hope it's not going to happen) and identifies a certain potential for improvement:

  1. Parameters currently is a mess of DBAL-level (host, port), wrapper-level (primary/replica, sharding), and driver-level (charset, memory) elements.
  2. There's no way to tell whether a given parameter is currently used or not (e.g. due to a typo or being driver/wrapper-specific).

Notes:

  1. The OverrideParams type is added temporarily until Psalm implements the support for self-referencing types (Internal Psalm error when handling self references in Psalm type vimeo/psalm#4653 (comment)).
  2. PHPStan currently interprets @psalm-* annotations but doesn't understand user-defined types. Therefore, each added annotation has to be doubled with a @phpstan- one with a simpler type.
  3. What the hell is default_dbname and why was it introduced in Added parameter default_dbname to pdo_pgsql driver, used to override the default database #2284?

@greg0ire
Copy link
Member

greg0ire commented Nov 25, 2020

What the hell is default_dbname and why was it introduced in #2284?

I think it might be the extra layer Postgres and only Postgres has. See https://stackoverflow.com/a/1157008/353612

I say might because I do not understand how to build a DSN saying to connect to schema foo into database bar with user baz, and the code in that PR does not help understanding that… they always modify dbname

@morozov
Copy link
Member Author

morozov commented Nov 25, 2020

I think it might be the extra layer Postgres and only Postgres has.

Most of the DB platforms have a notion of the schema (e.g. Oracle, SQL Server). In Oracle, the schema is a synonym of the user; in MySQL, it's a synonym of the database. DBAL already supports defining them separately. I'll file a deprecation PR since it doesn't make any sense. Most likely, the problem attempted to be solved can be solved w/o introducing an extra configuration parameter.

@morozov morozov merged commit 4b46ecd into doctrine:2.12.x Nov 25, 2020
@morozov morozov deleted the params-shape branch November 25, 2020 20:02
@morozov morozov added this to the 2.12.2 milestone Nov 25, 2020
@bendavies
Copy link
Contributor

i'm thinking @psalm-type Params is missing url. what do you think?

@morozov
Copy link
Member Author

morozov commented Apr 5, 2021

As far as I understand, nothing prevents it from being used, it's just of unknown type. Adding it shouldn't hurt.

@morozov morozov modified the milestones: 2.12.2, 2.13.0 Apr 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants