Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Fix AlterTable::dropConstraint() by allowing the use of decorator #247

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
18 changes: 11 additions & 7 deletions src/Sql/Ddl/AlterTable.php
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@
namespace Zend\Db\Sql\Ddl;

use Zend\Db\Adapter\Platform\PlatformInterface;
use Zend\Db\Metadata\Object\ConstraintObject;
use Zend\Db\Sql\AbstractSql;

class AlterTable extends AbstractSql implements SqlInterface
@@ -27,7 +28,7 @@ class AlterTable extends AbstractSql implements SqlInterface
protected $addColumns = [];

/**
* @var array
* @var Constraint\ConstraintInterface[]
*/
protected $addConstraints = [];

@@ -42,7 +43,7 @@ class AlterTable extends AbstractSql implements SqlInterface
protected $dropColumns = [];

/**
* @var array
* @var ConstraintObject[]
*/
protected $dropConstraints = [];

@@ -74,7 +75,7 @@ class AlterTable extends AbstractSql implements SqlInterface
],
self::DROP_CONSTRAINTS => [
"%1\$s" => [
[1 => "DROP CONSTRAINT %1\$s,\n", 'combinedby' => ""],
[2 => "DROP %1\$s %2\$s,\n", 'combinedby' => ""],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not modify generic syntax from abstract classes other adapters inherit from for engine specific fixes. Fixing one engine breaks assumptions of others within this library or others making custom adapters.

]
]
];
@@ -138,12 +139,12 @@ public function dropColumn($name)
}

/**
* @param string $name
* @param ConstraintObject $constraint
* @return self Provides a fluent interface
*/
public function dropConstraint($name)
public function dropConstraint(ConstraintObject $constraint)
{
$this->dropConstraints[] = $name;
$this->dropConstraints[] = $constraint;

return $this;
}
@@ -229,7 +230,10 @@ protected function processDropConstraints(PlatformInterface $adapterPlatform = n
{
$sqls = [];
foreach ($this->dropConstraints as $constraint) {
$sqls[] = $adapterPlatform->quoteIdentifier($constraint);
$sqls[] = [
'CONSTRAINT',
$adapterPlatform->quoteIdentifier($constraint->getName())
Copy link
Contributor

@alextech alextech May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until this #233 goes through that fixes bugs in quoteIdentifier vs quoteIdentifierChain this will break PostgreSQL and MSSQL. If lucky and that PR gets merged prior to this, then its fine.

];
}

return [$sqls];
13 changes: 13 additions & 0 deletions src/Sql/Platform/Mysql/Ddl/AlterTableDecorator.php
Original file line number Diff line number Diff line change
@@ -248,4 +248,17 @@ private function compareColumnOptions($columnA, $columnB)

return $columnA - $columnB;
}

protected function processDropConstraints(PlatformInterface $adapterPlatform = null)
{
$sqls = [];
foreach ($this->dropConstraints as $constraint) {
$sqls[] = [
$constraint->getType(),
$adapterPlatform->quoteIdentifier($constraint->getName())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above

];
}

return [$sqls];
}
}
8 changes: 5 additions & 3 deletions test/Sql/Ddl/AlterTableTest.php
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@

namespace ZendTest\Db\Sql\Ddl;

use Zend\Db\Metadata\Object\ConstraintObject;
use Zend\Db\Sql\Ddl\AlterTable;
use Zend\Db\Sql\Ddl\Column;
use Zend\Db\Sql\Ddl\Constraint;
@@ -66,8 +67,9 @@ public function testDropColumn()
public function testDropConstraint()
{
$at = new AlterTable();
$this->assertSame($at, $at->dropConstraint('foo'));
$this->assertEquals(['foo'], $at->getRawState($at::DROP_CONSTRAINTS));
$constraint = new ConstraintObject('foo', null);
$this->assertSame($at, $at->dropConstraint($constraint));
$this->assertEquals([$constraint], $at->getRawState($at::DROP_CONSTRAINTS));
}

/**
@@ -93,7 +95,7 @@ public function testGetSqlString()
$at->changeColumn('name', new Column\Varchar('new_name', 50));
$at->dropColumn('foo');
$at->addConstraint(new Constraint\ForeignKey('my_fk', 'other_id', 'other_table', 'id', 'CASCADE', 'CASCADE'));
$at->dropConstraint('my_index');
$at->dropConstraint(new ConstraintObject('my_index', null));
$expected =<<<EOS
ALTER TABLE "foo"
ADD COLUMN "another" VARCHAR(255) NOT NULL,
8 changes: 7 additions & 1 deletion test/Sql/Platform/Mysql/Ddl/AlterTableDecoratorTest.php
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@
namespace ZendTest\Db\Sql\Platform\Mysql\Ddl;

use Zend\Db\Adapter\Platform\Mysql;
use Zend\Db\Metadata\Object\ConstraintObject;
use Zend\Db\Sql\Ddl\AlterTable;
use Zend\Db\Sql\Ddl\Column\Column;
use Zend\Db\Sql\Ddl\Constraint\PrimaryKey;
@@ -45,8 +46,13 @@ public function testGetSqlString()
$col->addConstraint(new PrimaryKey());
$ct->addColumn($col);

$fk = new ConstraintObject('my_fk', null);
$fk->setType('FOREIGN KEY');
$ct->dropConstraint($fk);

$this->assertEquals(
"ALTER TABLE `foo`\n ADD COLUMN `bar` INTEGER UNSIGNED ZEROFILL NOT NULL AUTO_INCREMENT PRIMARY KEY COMMENT 'baz' AFTER `bar`",
"ALTER TABLE `foo`\n ADD COLUMN `bar` INTEGER UNSIGNED ZEROFILL NOT NULL AUTO_INCREMENT PRIMARY KEY COMMENT 'baz' AFTER `bar`,\n"
." DROP FOREIGN KEY `my_fk`",
@$ctd->getSqlString(new Mysql())
);
}