Escape primary field name in create table sql query #222

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@Genokilller

Fixes primary fields name in CreateTableSQL for Abstract Platform

Update lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
Fixes primary fields name in CreateTableSQL for Abstract Platform
@doctrinebot

This comment has been minimized.

Show comment
Hide comment
@doctrinebot

doctrinebot Oct 30, 2012

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-374

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-374

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Oct 30, 2012

Member

What are you trying to fix ? I don't see any testcase showing that a bug has been fixed and no reference to an existing ticket

Member

stof commented Oct 30, 2012

What are you trying to fix ? I don't see any testcase showing that a bug has been fixed and no reference to an existing ticket

@Genokilller

This comment has been minimized.

Show comment
Hide comment
@Genokilller

Genokilller Oct 30, 2012

This fixe is for a bug that a see when i want to create a new table with MySQL Driver.
I define a primary key named 'key'. The column name was correctly escape, but not the primary key definition.

Exemple :
Without fix: PRIMARY KEY(key)
With fix: PRIMARY KEY(key)

This is a reserved keyword for MySQL.

I don't have the time for writting some unit tests about that.

Thanks a lot.

This fixe is for a bug that a see when i want to create a new table with MySQL Driver.
I define a primary key named 'key'. The column name was correctly escape, but not the primary key definition.

Exemple :
Without fix: PRIMARY KEY(key)
With fix: PRIMARY KEY(key)

This is a reserved keyword for MySQL.

I don't have the time for writting some unit tests about that.

Thanks a lot.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Oct 30, 2012

Member

it should be escaped only when the column is marked as needing quotes IMO, as quoting column names introduces issues

Member

stof commented Oct 30, 2012

it should be escaped only when the column is marked as needing quotes IMO, as quoting column names introduces issues

@Genokilller

This comment has been minimized.

Show comment
Hide comment
@Genokilller

Genokilller Oct 30, 2012

The name is escape only if it's a reserved keyword.

You hope i listen this sentence and apply it instead of apply quoting ?
"Let me answer with another question: why on earth do you use a reserved keyword as identifier? ;-) "

So, why the columns name should be quoted and not the index ?

The name is escape only if it's a reserved keyword.

You hope i listen this sentence and apply it instead of apply quoting ?
"Let me answer with another question: why on earth do you use a reserved keyword as identifier? ;-) "

So, why the columns name should be quoted and not the index ?

@beberlei

This comment has been minimized.

Show comment
Hide comment
@beberlei

beberlei Nov 13, 2012

Member

Verified, i am working on a fix thtas different than this one though.

Member

beberlei commented Nov 13, 2012

Verified, i am working on a fix thtas different than this one though.

@beberlei

This comment has been minimized.

Show comment
Hide comment
@beberlei

beberlei Nov 13, 2012

Member

Fixed in master and 2.3

Member

beberlei commented Nov 13, 2012

Fixed in master and 2.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment