doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.92k stars 2.51k forks source link

validate for column/table names not being reserved keywords #6863

Open FyiurAmron opened 6 years ago

FyiurAmron commented 6 years ago

FWIW, there's a lot of StackOverflow/GitHub issues about people who used one of many SQL's reserved keywords in their ORMs:

https://stackoverflow.com/questions/20798240/doctrine2-sqlstate42000-syntax-error-or-access-violation-1064 https://stackoverflow.com/questions/35154203/sqlstate42000-syntax-error-or-access-violation-1064-symfony-2-7-3-applicatio https://stackoverflow.com/questions/23446377/syntax-error-due-to-using-a-reserved-word-as-a-table-or-column-name-in-mysql https://stackoverflow.com/questions/19147073/why-sqlstate42000-syntax-error-or-access-violation https://stackoverflow.com/questions/14437031/syntax-error-on-a-doctrine-query-symfony2 https://github.com/doctrine/common/issues/165 https://github.com/doctrine/doctrine2/issues/3281 https://github.com/nextcloud/server/issues/3140 etc. etc.

The list of SQL's reserved words https://www.drupal.org/docs/develop/coding-standards/list-of-sql-reserved-words is quite extensive, and some are extremely obscure. I've already hit three of those by accident in two days. The simplest solution IMO would be for Doctrine to actually make column/table names that are colliding with SQL's RW's simply fail validation. doctrine:schema:validate, "column name can't be an SQL reserved keyword; either change or quote with backticks" reported, et voila, instead of the ugly and cryptic "SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax".

Ocramius commented 6 years ago

The idea for 3.x is to break BC with case sensitivity issues and always quote identifiers, so the issue would disappear when that's implemented.

On 5 Dec 2017 05:58, "FyiurAmron" notifications@github.com wrote:

FWIW, there's a lot of StackOverflow/GitHub issues about people who used one of many SQL's reserved keywords in their ORMs:

https://stackoverflow.com/questions/20798240/doctrine2- sqlstate42000-syntax-error-or-access-violation-1064 https://stackoverflow.com/questions/35154203/ sqlstate42000-syntax-error-or-access-violation-1064-symfony- 2-7-3-applicatio https://stackoverflow.com/questions/23446377/syntax- error-due-to-using-a-reserved-word-as-a-table-or-column-name-in-mysql https://stackoverflow.com/questions/19147073/why- sqlstate42000-syntax-error-or-access-violation https://stackoverflow.com/questions/14437031/syntax- error-on-a-doctrine-query-symfony2 doctrine/common#165 https://github.com/doctrine/common/issues/165

3281 https://github.com/doctrine/doctrine2/issues/3281

nextcloud/server#3140 https://github.com/nextcloud/server/issues/3140 etc. etc.

The list of SQL's reserved words https://www.drupal.org/docs/ develop/coding-standards/list-of-sql-reserved-words is quite extensive, and some are extremely obscure. I've already hit three of those by accident in two days. The simplest solution IMO would be for Doctrine to actually make column/table names that are colliding with SQL's RW's simply fail validation. doctrine:schema:validate, "column name can't be an SQL reserved keyword; either change or quote with backticks" reported, et voila, instead of the ugly and cryptic "SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax".

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/doctrine/doctrine2/issues/6863, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJakNxlTUc_Hh0tbZZHzcw212APrZS2ks5s9M18gaJpZM4Q1vJB .

FyiurAmron commented 6 years ago

@Ocramius well, it's good hear that. Still, wouldn't issuing a warning be sensible even then? I'd say that the list of reserved keywords in SQL is a source of constant WTFs for people around the globe, so having even a rudimentary name validator built into Doctrine's validation would simplify the process anyway - so that people won't be surprised that their names, which would work perfectly in regular Doctrine 3 use cases, wouldn't work unquoted e.g. with raw SQL queries etc.

Ocramius commented 6 years ago

Yes, but 2.x is pretty much falling out of our focus now, as our (extremely limited) resources are set on implementing 3.x improvements and simplifications.

On 6 Dec 2017 17:25, "FyiurAmron" notifications@github.com wrote:

@Ocramius https://github.com/ocramius well, it's good hear that. Still, wouldn't issuing a warning be sensible even then? I'd say that the list of reserved keywords in SQL is a source of constant WTFs for people around the globe, so having a simple validator built into Doctrine would simplify the validation process anyway (so that people won't be surprised that their names, which would work perfectly in regular Doctrine 3 use cases, wouldn't work unquoted e.g. with raw SQL queries etc.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/doctrine/doctrine2/issues/6863#issuecomment-349692560, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJakCJPI8s6kkQLyvvHThe01JWFpnghks5s9r_bgaJpZM4Q1vJB .

Majkl578 commented 6 years ago

Still, wouldn't issuing a warning be sensible even then?

If 3.0 achieves full SQL compliance with auto-quoting, I don'tthink it's needed, it would be unnecessary to warn people against something that works. But it should be noted in documentation.

The list of SQL's reserved words

Just for precision, the list on Drupal site you referenced is not accurate, it is compiled all-into-one and also contains non-reserved keywords - not ideal. Keywords are platform- and version-specific, this is something where we can leverage DBAL's platform keywords.