Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-88

SchemaTool/Platform DDL does not escape column names

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-ALPHA2
    • Fix Version/s: 2.0-ALPHA3
    • Component/s: None
    • Security Level: All
    • Labels:
      None

      Description

      The column identifiers are not quoted in the SchemaTool/Platform.

      Funny is Doctrine\Tests\ORM\Functional\SchemaTool\MysqlSchemaToolTest::testGetCreateSchemaSql2() suffers from this "problem" by asserting mysql invalid syntax as correct creation code.

      All identifiers should be escaped by the Platform.

        Activity

        Hide
        Roman S. Borschel added a comment -

        This is actually intended, I mean not the wrong test case but the fact that the DBAL itself does not escape anything. If someone is using the DBAL directly he can quote problematic names himself ($platform->quoteIdentifier(...)). Identifier quoting is discouraged and not even a 100% reliable solution for reserved words.

        There is also no global switch anymore like in D1 to "quote all identifiers" as this is like breaking a butterfly on the wheel if there is just 1 problematic column name.

        How quoting a specific column works from the ORM is described here:

        http://www.doctrine-project.org/documentation/manual/2_0/en/basic-mapping#quoting-reserved-words

        So, the ORM applies quoting on individual columns that have been marked as such. Apart from that, there is no quoting going on, neither in the ORM, nor in the DBAL.

        The test case needs to be fixed, of course.

        Show
        Roman S. Borschel added a comment - This is actually intended, I mean not the wrong test case but the fact that the DBAL itself does not escape anything. If someone is using the DBAL directly he can quote problematic names himself ($platform->quoteIdentifier(...)). Identifier quoting is discouraged and not even a 100% reliable solution for reserved words. There is also no global switch anymore like in D1 to "quote all identifiers" as this is like breaking a butterfly on the wheel if there is just 1 problematic column name. How quoting a specific column works from the ORM is described here: http://www.doctrine-project.org/documentation/manual/2_0/en/basic-mapping#quoting-reserved-words So, the ORM applies quoting on individual columns that have been marked as such. Apart from that, there is no quoting going on, neither in the ORM, nor in the DBAL. The test case needs to be fixed, of course.
        Hide
        Benjamin Eberlei added a comment -

        But the DDL SQL is generated inside DBAL Platform, i don't see a way to affect its quoting identifiers or not strategy.

        All identifiers should be escaped by the platform maybe a bit harsh, i meant all the DDL sql.

        Show
        Benjamin Eberlei added a comment - But the DDL SQL is generated inside DBAL Platform, i don't see a way to affect its quoting identifiers or not strategy. All identifiers should be escaped by the platform maybe a bit harsh, i meant all the DDL sql.
        Hide
        Roman S. Borschel added a comment -

        If we would do that we would then need to take care of not quoting twice.

        I'll try to explain how it currently works, its quite simple. Yes, the platform constructs the DDL, based on some parameters and these may already be quoted or escaped or whatever. The platform doesnt do anything with it, except embed it into the DDL statement.

        I'll try to quickly outline the process of a quoted table/column name:

        1) @Column(name="`number`", ...)

        2) Inside ClassMetadata/AssociactionMapping where the mapping is validated & completed, the following happens: If the column name starts with a backtick `, apply trim('`', $name) before storing the name (never store quoted names anywhere!) and mark 'quoted' => true for that column.

        3) Whenever a table or column name is placed in an SQL statement *or* passed to the DBAL use one of the following methods, depending on who owns the column:

        ClassMetadata#getQuotedColumnName($name, $platform)
        ClassMetadata#getQuotedTableName($platform)
        OneToOneMapping#getQuotedJoinColumnName($name, $platform)
        ManyToManyMapping#getQuotedJoinColumnName($name, $platform)

        Thats it.

        Now, SchemaTool, for example uses (or should use!) These methods when it constructs the parameters that are passed to the DBAL.

        Identifier quoting is a very ugly problem and we tried to minimize the effect it has on our code. The only solid way to fix the problem of a reserved name is to change the name and not to quote it.

        Of course, if you have a better idea/solution, just shoot!

        Show
        Roman S. Borschel added a comment - If we would do that we would then need to take care of not quoting twice. I'll try to explain how it currently works, its quite simple. Yes, the platform constructs the DDL, based on some parameters and these may already be quoted or escaped or whatever. The platform doesnt do anything with it, except embed it into the DDL statement. I'll try to quickly outline the process of a quoted table/column name: 1) @Column(name="`number`", ...) 2) Inside ClassMetadata/AssociactionMapping where the mapping is validated & completed, the following happens: If the column name starts with a backtick `, apply trim('`', $name) before storing the name (never store quoted names anywhere!) and mark 'quoted' => true for that column. 3) Whenever a table or column name is placed in an SQL statement * or * passed to the DBAL use one of the following methods, depending on who owns the column: ClassMetadata#getQuotedColumnName($name, $platform) ClassMetadata#getQuotedTableName($platform) OneToOneMapping#getQuotedJoinColumnName($name, $platform) ManyToManyMapping#getQuotedJoinColumnName($name, $platform) Thats it. Now, SchemaTool, for example uses (or should use!) These methods when it constructs the parameters that are passed to the DBAL. Identifier quoting is a very ugly problem and we tried to minimize the effect it has on our code. The only solid way to fix the problem of a reserved name is to change the name and not to quote it. Of course, if you have a better idea/solution, just shoot!
        Hide
        Roman S. Borschel added a comment -

        Btw. What is the reserved word in testGetCreateSchemaSql2 ? "decimal" ?

        Show
        Roman S. Borschel added a comment - Btw. What is the reserved word in testGetCreateSchemaSql2 ? "decimal" ?
        Hide
        Benjamin Eberlei added a comment -

        Yes its decimal, thanks for your explanation, i get it why its not necessary to quote at this point then.

        Ok, then its probably just that decimal should be quoted in the DecimalModel.

        Show
        Benjamin Eberlei added a comment - Yes its decimal, thanks for your explanation, i get it why its not necessary to quote at this point then. Ok, then its probably just that decimal should be quoted in the DecimalModel.
        Hide
        Roman S. Borschel added a comment -

        Just did that! Thanks for finding this.

        Show
        Roman S. Borschel added a comment - Just did that! Thanks for finding this.

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Benjamin Eberlei
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: