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

QueryBuilder delete function should remove alias parameter? #2297

Closed
bitgandtter opened this issue Jan 13, 2016 · 7 comments
Closed

QueryBuilder delete function should remove alias parameter? #2297

bitgandtter opened this issue Jan 13, 2016 · 7 comments

Comments

@bitgandtter
Copy link

Since alias is available on the delete function the given SQL for delete can be in the form of:

DELETE FROM table t0 ...

That syntax will rise the following kind of error on MySQL or MariaDB:

Error: ER_PARSE_ERROR: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 't0 WHERE ' at line 1

To avoid this, the delete function should remove the alias parameter and force alias null on "add" function

@deeky666
Copy link
Member

@bitgandtter I don't quite get the issue here. The QueryBuilder does not generate aliases in SQL if you omit it. See here.
Also if MySQL does not support table aliases in DELETE statements, simply don't use them. Other RDBMs support it. As QueryBuilder is designed for simple queries and considering it is platform agnostic, we won't start implementing DELETE statement abstraction in 2.x.

@marcinkleczek
Copy link

MySQL support table alias on DELETE statement, but you need to determine from which table deletion must be done:
DELETE p.* FROM products p WHERE p.name = 'asdf';

@deeky666
Copy link
Member

Okay but I still don't see the issue here. @bitgandtter please clarify why you can't simply omit the table alias in the QueryBuilder. Thanks.

@bitgandtter
Copy link
Author

Yes, you can omit the alias and it will work but since you can make the mistake and set it, it will not be better to prevent this kind of issue? Since as you say its database agnostic and all databases support the "non alias" statement and only MySQL and MariaDB by consequence does not support it, should not be better to allow "non aliases" delete only?

@deeky666
Copy link
Member

@bitgandtter we cannot simply force remove the aliases as it might break existing applications. There might be Queries where you'll need aliases (can't get my head around this now) so having it opt-in is totally fine IMO. I see no benefit here in making the API less flexible only to help some MySQL users avoid making mistakes. It simply is API misusage. Closing.

@Wirone
Copy link

Wirone commented Sep 7, 2016

I was struggling for a while with this and I think it's kind of issue. Of course I don't think DBAL should prevent using alias in DELETE statements, but maybe QueryBuilder::getSQLForDelete() could use $this->connection->getDatabasePlatform(), detect if it's MySQL and use alias if provided?

Of course alias can be ommited, but why don't handle situation when it's set, since it's possible?

@github-actions
Copy link

github-actions bot commented Aug 7, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2022
@morozov morozov closed this as not planned Won't fix, can't repro, duplicate, stale Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants