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

table()->join()->where()->delete() generates incorrect SQL using Query Builder #16573

Closed
cbj4074 opened this issue Nov 28, 2016 · 9 comments
Closed

Comments

@cbj4074
Copy link
Contributor

cbj4074 commented Nov 28, 2016

  • Laravel Version: 5.1.45
  • PHP Version: 7.0.13
  • Database Driver & Version: PostgreSQL(libpq) Version 9.5.5

@benubird reported this in the form of a PR, back in 2014:

#5652

The PR was submitted to the wrong branch and later closed (presumably because the submitter never followed-up).

Relative to some of Query Builder's other feats, this seems like pretty basic functionality that should be implemented.

@sporchia submitted a subsequent PR, #5703 , which appears to have addressed the same issue in Eloquent, so perhaps the same logic could be applied to Query Builder.

@fernandobandeira
Copy link
Contributor

fernandobandeira commented Nov 29, 2016

It seems that this is fixed for MySQL.

SqlServer supports inner joins so we could add this to the grammar.

PostgreSQL doesn't support inner joins on delete but it supports using, we could try to work around or add support for the using syntax (MySQL also supports this syntax),

SQLite supports only subqueries.

@srmklive
Copy link
Contributor

@cbj4074 #5703 was later reverted in #5711.

@srmklive
Copy link
Contributor

srmklive commented Nov 29, 2016

@fernandobandeira The change you are suggesting might break the Query Builder implementation for Postgres. That's my only worry.

@fernandobandeira
Copy link
Contributor

Yeah we need to treat sqlite and Postgres differently, #5703 was moved to affect just MySQL as it broke Postgres #5703 (comment)

I moved it to the MySQL grammar.

@cbj4074
Copy link
Contributor Author

cbj4074 commented Dec 2, 2016

Thanks so much for taking a look, @srmklive and @fernandobandeira .

Excellent point regarding the lack of support for inner joins with delete in PostgreSQL.

I lack any direct experience with the grammar engine... are you comfortable implementing this, @fernandobandeira ? I'd be most appreciative. 👍

@fernandobandeira
Copy link
Contributor

Yes i'll take a look into it, but the using syntax is also used in MySQL and it behaves differently on selects and update so it's something that should be implemented with care, but I should create a PR for 5.4, i've created one to add the support for joins on Sqlserver, so we will support the joins in all possible drivers, this issue will turn into a feature request after that merge, not sure if it should remain open... Maybe we should create one on the internals and close this one 😄 ...

@themsaid
Copy link
Member

themsaid commented Mar 2, 2017

I wonder if #18156 fixes this issue?

@fernandobandeira
Copy link
Contributor

@themsaid This one is actually a feature request to include support for joins when deleting using the postgres driver, currently we don't support it, we can't support it since postgres doesn't allow you to use joins on delete queries... IMO we can close this since it's not a bug and we can't really add support for it...

@themsaid
Copy link
Member

themsaid commented Mar 2, 2017

Thank you @fernandobandeira

@themsaid themsaid closed this as completed Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants