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

When rendering SQL, only render the alias if it's different from the table name #3045

Merged
merged 1 commit into from
Mar 15, 2018

Conversation

morozov
Copy link
Member

@morozov morozov commented Mar 13, 2018

When DBAL is used as part of a higher level abstraction where queries are built dynamically, the table name and the alias in SELECT, UPDATE and DELETE queries may be defined independently and may end up being the same. In this case, the resulting SQL contains a redundant alias:

$builder = new QueryBuilder($this->conn);
$builder->select('id')
    ->from('users', 'users');

// SELECT id FROM users users

…which is technically valid but not needed. In order to avoid that, the caller needs to compare $table and $alias before calling $builder->from() (in the case above) and pass null as the alias in the case if they are equal.

The caller logic could be simplified without an impact on generated SQL if the DBAL itself only rendered the alias if it's different from the table name.

@Ocramius Ocramius self-assigned this Mar 15, 2018
@Ocramius Ocramius modified the milestones: 2.7.0, 3.0.0 Mar 15, 2018
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

👍

@Ocramius Ocramius merged commit e6e2eea into doctrine:develop Mar 15, 2018
@morozov morozov deleted the remove-redundant-alias branch April 19, 2018 20:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 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.

2 participants