Add keyword aware quote strategy #5903
Conversation
For integration of doctrine/orm#5903 into DoctrineBundle.
|
@guilhermeblanco can you take a look at this? |
|
@Deltachaos tests are missing tho |
|
@Ocramius will add tests as soon as is see that there is a chance that this gets merged |
| $keywords = $platform->getReservedKeywordsList(); | ||
| $parts = explode(".", $name); | ||
| foreach ($parts as $k => $v) { | ||
| $parts[$k] = ($force || $keywords->isKeyword($v)) ? $platform->quoteIdentifier($v) : $v; |
There was a problem hiding this comment.
Unnecessary write of the unmodified value here, rather use normal if.
|
It'd be really good to have a native way to quote reserved words. 👍 |
| * @since 2.6 | ||
| * @author Maximilian Ruta <mr@xtain.net> | ||
| */ | ||
| class KeywordQuoteStrategy implements QuoteStrategy |
There was a problem hiding this comment.
I think keyword may be a bit misleading, what about e.g. ReservedWordQuoteStrategy?
For integration of doctrine/orm#5903 into DoctrineBundle.
Deltachaos
force-pushed
the
keyword-qoute
branch
from
3bdec22 to
cb493f0
Compare
Deltachaos
force-pushed
the
keyword-qoute
branch
from
cb493f0 to
4da9b3c
Compare
|
@Majkl578 tests are added and class was renamed |
|
@Deltachaos Do you still plan on fixing the broken tests? |
|
@mikeSimonson yes i have this still on my list. But i can not guarantee to fix this within the next month. Probably start of next year. |
|
@Deltachaos Thanks |
|
Quoting is deeply wtf in DBAL - 3.0 needs to address more than just reserved word quoting... |
Deltachaos
force-pushed
the
keyword-qoute
branch
from
dce4d7d to
a14e778
Compare
|
@Deltachaos I think we're going for quoting-by-default in ORM 3.x, so I'm not sure if this will be required by then... /cc @guilhermeblanco just to confirm. |
|
@Ocramius everything is quoted (and cannot be turned off) in |
|
Closing as |
Related to the discussion in doctrine/dbal#1592 it is probably a good idea to add this quote strategy. It can be enabled by the user optionally and therefore he should be aware of the upercase/lowercase issue in some DBMS.
For the integration of this into doctrine-bundle see this PR: doctrine/DoctrineBundle#553