-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DDC-1845] QuoteStrategy #372
Conversation
public function getQuoteStrategy() | ||
{ | ||
if ( ! isset($this->_attributes['quoteStrategy'])) { | ||
$this->_attributes['quoteStrategy'] = new \Doctrine\ORM\Mapping\DefaultQuoteStrategy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should add a use statement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FabioBatSilva you forgot this one ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done ;)
DONE !!! Thanks @stof |
please fix the code for PostgreSQL and SQLite |
if (empty($joinColumn['referencedColumnName'])) { | ||
$joinColumn['referencedColumnName'] = $this->namingStrategy->referenceColumnName(); | ||
} | ||
|
||
if ($joinColumn['name'][0] == '`') { | ||
$joinColumn['name'] = trim($joinColumn['name'], '`'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is obviously for MySQL: are there other engines using different characters for quoting identifiers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @Ocramius
This is just a doctrine quote token, not used in SQL level
Hi @stof already is fixed. PostgreSQL fails on cache, even in doctrine/master. not related with my patch. |
+1 from me, @guilhermeblanco whats your take? |
$class->getTableName() . '_' . $class->columnNames[$class->identifier[0]] . '_seq' : | ||
null; | ||
$class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator($seqName)); | ||
$sequenceName = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing line break between assignment and control structure
Hi @guilhermeblanco, Fixed !! Thanks |
$class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator($seqName)); | ||
$sequenceName = null; | ||
|
||
if($this->targetPlatform instanceof Platforms\PostgreSQLPlatform) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space between if
and open parenthesis
DONE !!! |
@FabioBatSilva are the failures now caused by the PR? They're not matching the ones I used to see on PostgreSQL... |
Hi @Ocramius PostgreSQL fails on cache, even in doctrine/master. not related with my patch. Thanks . |
good to know =) |
[DDC-1845] QuoteStrategy
QuoteStrategy
http://www.doctrine-project.org/jira/browse/DDC-1845
This patch fix some quote problems using a default quote strategy and allows users find solutions themselves for weird quote cases.
This DBAL PR shoud be merged to fix sqlite tests in : doctrine/dbal#158
There is a lote of new method calls, the performance tests in sqlite are the following :
My branch :
Doctrine Master :