[DDC-88] SchemaTool/Platform DDL does not escape column names Created: 31/Oct/09  Updated: 01/Nov/09  Resolved: 01/Nov/09

Status: Closed
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: 2.0-ALPHA2
Fix Version/s: 2.0-ALPHA3
Security Level: All

Type: Bug Priority: Major
Reporter: Benjamin Eberlei Assignee: Roman S. Borschel
Resolution: Fixed Votes: 0
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.



 Comments   
Comment by Roman S. Borschel [ 01/Nov/09 ]

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.

Comment by Benjamin Eberlei [ 01/Nov/09 ]

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.

Comment by Roman S. Borschel [ 01/Nov/09 ]

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!

Comment by Roman S. Borschel [ 01/Nov/09 ]

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

Comment by Benjamin Eberlei [ 01/Nov/09 ]

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.

Comment by Roman S. Borschel [ 01/Nov/09 ]

Just did that! Thanks for finding this.

Generated at Tue Sep 02 05:21:00 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.