Skip to content
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

Ensuring correct ALTER TABLE statement for creation of an AUTO INCREMENT column as new PRIMARY KEY #3310

Closed
wants to merge 11 commits into from
10 changes: 5 additions & 5 deletions .github/ISSUE_TEMPLATE/BC_Break.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: 💥 BC Break
about: Have you encountered an issue during upgrade? 💣
about: Have you encountered an issue during an upgrade? 💣
---

<!--
Expand All @@ -18,15 +18,15 @@ Before reporting a BC break, please consult the upgrading document to make sure

#### Summary

<!-- Provide a summary desciribing the problem you are experiencing. -->
<!-- Provide a summary describing the problem you are experiencing. -->

#### Previous behavior
#### Previous behaviour

<!-- What was the previous (working) behavior? -->
<!-- What was the previous (working) behaviour? -->

#### Current behavior

<!-- What is the current (broken) behavior? -->
<!-- What is the current (broken) behaviour? -->

#### How to reproduce

Expand Down
8 changes: 4 additions & 4 deletions .github/ISSUE_TEMPLATE/Bug.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ about: Something is broken? 🔨

#### Summary

<!-- Provide a summary desciribing the problem you are experiencing. -->
<!-- Provide a summary describing the problem you are experiencing. -->

#### Current behavior

<!-- What is the current (buggy) behavior? -->
<!-- What is the current (buggy) behaviour? -->

#### How to reproduce

Expand All @@ -28,7 +28,7 @@ If possible, also add a code snippet with relevant configuration, driver/platfor
Adding a failing Unit or Functional Test would help us a lot - you can submit one in a Pull Request separately, referencing this bug report.
-->

#### Expected behavior
#### Expected behaviour

<!-- What was the expected (correct) behavior? -->
<!-- What was the expected (correct) behaviour? -->

2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/Support_Question.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ about: Have a problem that you can't figure out? 🤔
| Version | x.y.z

<!--
Before asking question here, please try asking on Gitter or Slack first.
Before asking a question here, please try asking on Gitter or Slack first.
Find out more about Doctrine support channels here: https://www.doctrine-project.org/community/
Keep in mind that GitHub is primarily an issue tracker.
-->
Expand Down
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@

#### Summary

<!-- Provide a summary your change. -->
<!-- Provide a summary of your change. -->
12 changes: 12 additions & 0 deletions lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,18 @@ public function getAlterTableSQL(TableDiff $diff)
$queryParts[] = 'ADD PRIMARY KEY (' . implode(', ', $keyColumns) . ')';
unset($diff->addedIndexes['primary']);
}
// Necessary in case the new primary key includes a new auto_increment column
elseif (isset($diff->changedIndexes['primary'])) {
foreach ($diff->changedIndexes['primary']->getColumns() as $columnName) {
if (isset($diff->addedColumns[$columnName]) && $diff->addedColumns[$columnName]->getAutoincrement()) {
$keyColumns = array_unique(array_values($diff->changedIndexes['primary']->getColumns()));
$queryParts[] = 'DROP PRIMARY KEY';
$queryParts[] = 'ADD PRIMARY KEY (' . implode(', ', $keyColumns) . ')';
unset($diff->changedIndexes['primary']);
break;
}
}
}

$sql = [];
$tableSql = [];
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/SQLParserUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class SQLParserUtils
const ESCAPED_SINGLE_QUOTED_TEXT = "(?:'(?:\\\\\\\\)+'|'(?:[^'\\\\]|\\\\'?|'')*')";
const ESCAPED_DOUBLE_QUOTED_TEXT = '(?:"(?:\\\\\\\\)+"|"(?:[^"\\\\]|\\\\"?)*")';
const ESCAPED_BACKTICK_QUOTED_TEXT = '(?:`(?:\\\\\\\\)+`|`(?:[^`\\\\]|\\\\`?)*`)';
const ESCAPED_BRACKET_QUOTED_TEXT = '(?<!\bARRAY)\[(?:[^\]])*\]';
private const ESCAPED_BRACKET_QUOTED_TEXT = '(?<!\b(?i:ARRAY))\[(?:[^\]])*\]';

/**
* Gets an array of the placeholders in an sql statements as keys and their positions in the query string.
Expand Down
8 changes: 4 additions & 4 deletions lib/Doctrine/DBAL/Schema/Sequence.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ class Sequence extends AbstractAsset
public function __construct($name, $allocationSize = 1, $initialValue = 1, $cache = null)
{
$this->_setName($name);
$this->allocationSize = is_numeric($allocationSize) ? $allocationSize : 1;
$this->initialValue = is_numeric($initialValue) ? $initialValue : 1;
$this->setAllocationSize($allocationSize);
$this->setInitialValue($initialValue);
$this->cache = $cache;
}

Expand Down Expand Up @@ -93,7 +93,7 @@ public function getCache()
*/
public function setAllocationSize($allocationSize)
{
$this->allocationSize = is_numeric($allocationSize) ? $allocationSize : 1;
$this->allocationSize = is_numeric($allocationSize) ? (int) $allocationSize : 1;

return $this;
}
Expand All @@ -105,7 +105,7 @@ public function setAllocationSize($allocationSize)
*/
public function setInitialValue($initialValue)
{
$this->initialValue = is_numeric($initialValue) ? $initialValue : 1;
$this->initialValue = is_numeric($initialValue) ? (int) $initialValue : 1;

return $this;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Version.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Version
/**
* Current Doctrine Version.
*/
public const VERSION = '2.8.0-DEV';
public const VERSION = '2.8.1-DEV';

/**
* Compares a Doctrine version with the current one.
Expand Down
56 changes: 56 additions & 0 deletions tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL2807Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

namespace Doctrine\Tests\DBAL\Functional\Ticket;

use Doctrine\DBAL\Schema\Comparator;
use Doctrine\DBAL\Schema\Table;
use Doctrine\Tests\DbalFunctionalTestCase;

final class DBAL2807Test extends DbalFunctionalTestCase
{
/**
* {@inheritDoc}
*/
public function setUp()
{
parent::setUp();

if (!in_array($this->getPlatform()->getName(), ['mysql']))
{
$this->markTestSkipped('Restricted to MySQL.');

return;
}
}

/**
* Ensures that the primary key is created within the same "alter table" statement that an auto-increment column
* is added to the table as part of the new primary key.
*
* Before the fix for this problem this resulted in a database error:
* SQLSTATE[42000]: Syntax error or access violation: 1075 Incorrect table definition; there can be only one auto column and it must be defined as a key
*/
public function testAlterPrimaryKeyToAutoIncrementColumn()
{
$table = new Table('my_table');
$table->addColumn('initial_id', 'integer');
$table->setPrimaryKey(['initial_id']);

$newTable = clone $table;
$newTable->addColumn('new_id', 'integer', ['autoincrement' => true]);
$newTable->dropPrimaryKey();
$newTable->setPrimaryKey(['new_id']);

$diff = (new Comparator())->diffTable($table, $newTable);

$this->assertSame(
['ALTER TABLE my_table ADD new_id INT AUTO_INCREMENT NOT NULL, DROP PRIMARY KEY, ADD PRIMARY KEY (new_id)',],
$this->getPlatform()->getAlterTableSQL($diff)
);
}

protected function getPlatform()
{
return $this->_conn->getDatabasePlatform();
}
}
3 changes: 2 additions & 1 deletion tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ public function dataGetPlaceholderPositions()
array('SELECT foo::date as date FROM Foo WHERE bar > :start_date AND baz > :start_date', false, array(46 => 'start_date', 68 => 'start_date')), // Ticket GH-259
array('SELECT `d.ns:col_name` FROM my_table d WHERE `d.date` >= :param1', false, array(57 => 'param1')), // Ticket DBAL-552
array('SELECT [d.ns:col_name] FROM my_table d WHERE [d.date] >= :param1', false, array(57 => 'param1')), // Ticket DBAL-552
array('SELECT * FROM foo WHERE jsonb_exists_any(foo.bar, ARRAY[:foo])', false, array(56 => 'foo')), // Ticket GH-2295
['SELECT * FROM foo WHERE jsonb_exists_any(foo.bar, ARRAY[:foo])', false, [56 => 'foo']], // Ticket GH-2295
['SELECT * FROM foo WHERE jsonb_exists_any(foo.bar, array[:foo])', false, [56 => 'foo']],
array(
<<<'SQLDATA'
SELECT * FROM foo WHERE
Expand Down
2 changes: 2 additions & 0 deletions tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,13 @@ public function testCompareSequences()
$seq1 = new Sequence('foo', 1, 1);
$seq2 = new Sequence('foo', 1, 2);
$seq3 = new Sequence('foo', 2, 1);
$seq4 = new Sequence('foo', '1', '1');

$c = new Comparator();

self::assertTrue($c->diffSequence($seq1, $seq2));
self::assertTrue($c->diffSequence($seq1, $seq3));
self::assertFalse($c->diffSequence($seq1, $seq4));
}

public function testRemovedSequence()
Expand Down