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

Avoid ambigous TABLE_NAME in query #6275

Merged
merged 6 commits into from
Feb 1, 2024
Merged

Avoid ambigous TABLE_NAME in query #6275

merged 6 commits into from
Feb 1, 2024

Conversation

dsentker
Copy link
Contributor

Q A
Type bug
Fixed issues #6274

Summary

The current produced query leads to an ambigous column reference as described in the linked issue.

@derrabus derrabus added this to the 3.7.4 milestone Jan 25, 2024
@derrabus
Copy link
Member

Thank you. Can you try to add a functional test that reproduces your issue?

@dsentker
Copy link
Contributor Author

I just found out why the error occurred. My Symfony configuration stated that MariaDB 10 is being used. The actual DB server was MySQL 8.

I'll take a wild guess: MariaDB isn't that strict with ambiguous column names. Therefore, the error was not noticed from other devs with MariaDB before. MySQL 8 seems to be more strict.

The question is whether this should be called a bug. The actual mistake was mine (I tricked the configuration into MariaDB even though MySQL was on the other end.). What you might want to consider is that MariaDB will also dislike ambiguous column names more in the future.

@derrabus
Copy link
Member

Yeah, it's probably not a bug then (and cannot be tested in our CI), but I agree that making the query less ambiguous might be a good call. Can you have a look at my comment (#6275 (comment))? After that, the PR is ready to be merged, imho.

@dsentker
Copy link
Contributor Author

Yes, your alias insertion seems good to me.
Another idea: We should throw an InvalidArgumentException if 't' alias is used as method argument (to avoid conflicts).

Should i create new PR for the 3.8.x branch?

@derrabus
Copy link
Member

Should i create new PR for the 3.8.x branch?

No need for a new PR. Just do a rebase to solve the conflict.

@dsentker dsentker requested a review from derrabus January 26, 2024 14:56
@dsentker dsentker requested a review from derrabus January 26, 2024 15:05
@derrabus derrabus merged commit a9cd978 into doctrine:3.8.x Feb 1, 2024
90 checks passed
derrabus added a commit to derrabus/dbal that referenced this pull request Feb 3, 2024
* 3.8.x:
  Avoid ambigous TABLE_NAME in query (doctrine#6275)
  Add safety check about cache value (doctrine#6283)
derrabus added a commit to derrabus/dbal that referenced this pull request Feb 3, 2024
* 3.8.x:
  Avoid ambigous TABLE_NAME in query (doctrine#6275)
  Add safety check about cache value (doctrine#6283)
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.

2 participants