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

Optimize introspection of table comments on PostgreSQL #5449

Merged
merged 3 commits into from
Jun 19, 2022

Conversation

morozov
Copy link
Member

@morozov morozov commented Jun 18, 2022

Q A
Type improvement

The current implementation of PostgreSQLSchemaManager::fetchTableOptionsByTable() fetches the comment of each table individually although the API introduced in #5268 enables fetching schema metadata in a fixed number of queries.

Additionally, some cleanup is done in the codebase.

Comment on lines +729 to +730
* @param string|null $tableName
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param string|null $tableName
*

*/
private function getTableWhereClause($table, $classAlias = 'c', $namespaceAlias = 'n'): string
private function buildQueryConditions($tableName): array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private function buildQueryConditions($tableName): array
private function buildQueryConditions(?string $tableName): array

Copy link
Member Author

@morozov morozov Jun 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. We don't have strict types enabled here, so even given that the public methods that call this one don't enforce the type of $tableName, we can rely on implicit casting here.

But this is non-essential and is hard to reason about since these requirements aren't enforced by the code style or static analysis. We do want to enforce parameter types on all new APIs but it's not a public API.

This change will be consistently implemented during the merge up to 4.0.x.

@morozov morozov merged commit 84312ed into doctrine:3.4.x Jun 19, 2022
@morozov morozov deleted the pgsql-table-options branch June 19, 2022 14:49
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2023
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

2 participants