Doctrine DBAL
  1. Doctrine DBAL
  2. DBAL-40

Transparent table&column names escaping

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.3, 2.4, 2.4.1
    • Fix Version/s: None
    • Component/s: Platforms
    • Labels:
      None

      Description

      Hello, I would like to re-open the discussion about automatic transparent escaping of all table/column names sent from DBAL to database. It was already discussed in http://www.doctrine-project.org/jira/browse/DDC-88 without any satisfactory result.

      Why do I have to quote any reserved word used in table or column name? Why Doctrine doesn't do this automatically for all table and column
      names used in generated SQL queries?

      Before you start to explain how complicated it is and what problems you will be faced with, try to look at excellent DIBI database layer - how it acts in this way - it's behaviour is very cool. Unfortunally at the moment the full documentation is in czech only, but here is a brief automatic google-translation to english - http://dibiphp.com/en/quick-start.

      My suggestion to Doctrine 2 ORM/DBAL solution is:

      1. Developer should never care about any escaping or avoiding any reserved words - it is not his business, the DBAL shoult solve it transparently and safely.

      2. So there should be no need and even no possibility to add any quotation chars in @column or @table annotations as well as in DQL queries. ORM layer has nothing to do with escaping, it is all a business of the DBAL layer. Current possibility for manual escaping the names in mentioned annotations is totally wrong and should be discontinued.

      3. DBAL should escape ALL table and column names transparently and automatically. There should be ne option to enable or disable the escaping, there is no reason for disabling it.

      4. The escaping should be performed just in the final translation of DBAL queries to native SQL query, not earlier. This is the right place to do that.

      So what do you think about that?

        Issue Links

          Activity

          Hide
          Marco Pivetta added a comment -

          If you want quoting by default on everything we have a quoting strategy (in ORM) that you can use. I don't think quoting everything by default is a viable solution. Back in `Zend_Db` times this was eating up a lot of performance for no real reason. Users having a clean schema without horrors like columns called `order` or `group` should not be penalized because of users not using valid naming schemes.

          Show
          Marco Pivetta added a comment - If you want quoting by default on everything we have a quoting strategy (in ORM) that you can use. I don't think quoting everything by default is a viable solution. Back in `Zend_Db` times this was eating up a lot of performance for no real reason. Users having a clean schema without horrors like columns called `order` or `group` should not be penalized because of users not using valid naming schemes.
          Hide
          Steve Müller added a comment -

          Hello, if I understand correctly, the issue of quoting reserved keywords automatically is solved in https://github.com/doctrine/dbal/pull/302. Besides reserved keywords you can still decide quoting or not quoting identifier manually by passing quotes to the identifier or not.

          Show
          Steve Müller added a comment - Hello, if I understand correctly, the issue of quoting reserved keywords automatically is solved in https://github.com/doctrine/dbal/pull/302 . Besides reserved keywords you can still decide quoting or not quoting identifier manually by passing quotes to the identifier or not.
          Hide
          Arthur Bodera added a comment - - edited

          It's still broken in 2.4.

          PR 302 only selectively fixes indexes, PK and FK, but ALTER and all CRUD will still fail (and schema tool will produce invalid sql).

          There is no performance hit, as all operations already hit `DefaultQuoteStrategy`.

          Currently you have the following workarounds:

          • selectively add `quoted=true` to table and column names (ugh)
          • replace `DefaultQuoteStrategy` with strategy that quotes all identifiers.

          Here is a class you can use: https://gist.github.com/Thinkscape/6713196

          Show
          Arthur Bodera added a comment - - edited It's still broken in 2.4. PR 302 only selectively fixes indexes, PK and FK, but ALTER and all CRUD will still fail (and schema tool will produce invalid sql). There is no performance hit , as all operations already hit `DefaultQuoteStrategy`. Currently you have the following workarounds: selectively add `quoted=true` to table and column names (ugh) replace `DefaultQuoteStrategy` with strategy that quotes all identifiers. Here is a class you can use: https://gist.github.com/Thinkscape/6713196
          Hide
          Arthur Bodera added a comment -

          QuoteStrategies are not used for ALTER queries. This means that using the EagerQuoteStrategy mentioned above won't fix invalid ALTER queries generated by schema tool.

          For ALTER to work, we need this merged:

          https://github.com/doctrine/dbal/pull/379

          Show
          Arthur Bodera added a comment - QuoteStrategies are not used for ALTER queries. This means that using the EagerQuoteStrategy mentioned above won't fix invalid ALTER queries generated by schema tool. For ALTER to work, we need this merged: https://github.com/doctrine/dbal/pull/379
          Hide
          Doctrine Bot added a comment -

          A related Github Pull-Request [GH-379] was closed:
          https://github.com/doctrine/dbal/pull/379

          Show
          Doctrine Bot added a comment - A related Github Pull-Request [GH-379] was closed: https://github.com/doctrine/dbal/pull/379

            People

            • Assignee:
              Benjamin Eberlei
              Reporter:
              Jan Tichý
            • Votes:
              12 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated: