From 686b9c2c8bc0d4636ea0d0e5abc040bae90e4156 Mon Sep 17 00:00:00 2001
From: mark_story <mark@mark-story.com>
Date: Sun, 15 Aug 2010 22:17:02 -0400
Subject: [PATCH] Fixing issues in alterSchema in DboMysql and DboPostgres,
 where fields would be appended to each table being altered. Also fixed an
 issue in DboPostgres where the generated alter statements would contain too
 many ;. Test cases added.  Fixes #1023

---
 cake/libs/model/datasources/dbo/dbo_mysql.php |  2 +-
 .../model/datasources/dbo/dbo_postgres.php    |  4 +--
 .../model/datasources/dbo/dbo_mysql.test.php  | 34 ++++++++++++++++++
 .../datasources/dbo/dbo_postgres.test.php     | 35 +++++++++++++++++++
 4 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/cake/libs/model/datasources/dbo/dbo_mysql.php b/cake/libs/model/datasources/dbo/dbo_mysql.php
index 3e6681d9d..7b0bfea9b 100644
--- a/cake/libs/model/datasources/dbo/dbo_mysql.php
+++ b/cake/libs/model/datasources/dbo/dbo_mysql.php
@@ -286,7 +286,7 @@ class DboMysqlBase extends DboSource {
 		$out = '';
 		$colList = array();
 		foreach ($compare as $curTable => $types) {
-			$indexes = $tableParameters = array();
+			$indexes = $tableParameters = $colList = array();
 			if (!$table || $table == $curTable) {
 				$out .= 'ALTER TABLE ' . $this->fullTableName($curTable) . " \n";
 				foreach ($types as $type => $column) {
diff --git a/cake/libs/model/datasources/dbo/dbo_postgres.php b/cake/libs/model/datasources/dbo/dbo_postgres.php
index 49dd42cc8..6c692d942 100644
--- a/cake/libs/model/datasources/dbo/dbo_postgres.php
+++ b/cake/libs/model/datasources/dbo/dbo_postgres.php
@@ -562,7 +562,7 @@ class DboPostgres extends DboSource {
 		$out = '';
 		$colList = array();
 		foreach ($compare as $curTable => $types) {
-			$indexes = array();
+			$indexes = $colList = array();
 			if (!$table || $table == $curTable) {
 				$out .= 'ALTER TABLE ' . $this->fullTableName($curTable) . " \n";
 				foreach ($types as $type => $column) {
@@ -630,7 +630,7 @@ class DboPostgres extends DboSource {
 				} else {
 					$out = '';
 				}
-				$out .= implode(";\n\t", $this->_alterIndexes($curTable, $indexes)) . ";";
+				$out .= implode(";\n\t", $this->_alterIndexes($curTable, $indexes));
 			}
 		}
 		return $out;
diff --git a/cake/tests/cases/libs/model/datasources/dbo/dbo_mysql.test.php b/cake/tests/cases/libs/model/datasources/dbo/dbo_mysql.test.php
index 7c2def568..5a1bb4619 100644
--- a/cake/tests/cases/libs/model/datasources/dbo/dbo_mysql.test.php
+++ b/cake/tests/cases/libs/model/datasources/dbo/dbo_mysql.test.php
@@ -698,6 +698,40 @@ class DboMysqlTest extends CakeTestCase {
 		$this->db->query($this->db->dropSchema($schema1));
 	}
 
+/**
+ * test alterSchema on two tables.
+ *
+ * @return void
+ */
+	function testAlteringTwoTables() {
+		$schema1 =& new CakeSchema(array(
+			'name' => 'AlterTest1',
+			'connection' => 'test_suite',
+			'altertest' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'name' => array('type' => 'string', 'null' => false, 'length' => 50),
+			),
+			'other_table' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'name' => array('type' => 'string', 'null' => false, 'length' => 50),
+			)
+		));
+		$schema2 =& new CakeSchema(array(
+			'name' => 'AlterTest1',
+			'connection' => 'test_suite',
+			'altertest' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'field_two' => array('type' => 'string', 'null' => false, 'length' => 50),
+			),
+			'other_table' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'field_two' => array('type' => 'string', 'null' => false, 'length' => 50),
+			)
+		));
+		$result = $this->db->alterSchema($schema2->compare($schema1));
+		$this->assertEqual(2, substr_count($result, 'field_two'), 'Too many fields');
+	}
+
 /**
  * testReadTableParameters method
  *
diff --git a/cake/tests/cases/libs/model/datasources/dbo/dbo_postgres.test.php b/cake/tests/cases/libs/model/datasources/dbo/dbo_postgres.test.php
index 693cd66f7..ad7b3166b 100644
--- a/cake/tests/cases/libs/model/datasources/dbo/dbo_postgres.test.php
+++ b/cake/tests/cases/libs/model/datasources/dbo/dbo_postgres.test.php
@@ -824,4 +824,39 @@ class DboPostgresTest extends CakeTestCase {
 		));
 		$this->assertEqual($result, 3, 'Article count is wrong or fixture has changed.');
 	}
+
+/**
+ * test alterSchema on two tables.
+ *
+ * @return void
+ */
+	function testAlteringTwoTables() {
+		$schema1 =& new CakeSchema(array(
+			'name' => 'AlterTest1',
+			'connection' => 'test_suite',
+			'altertest' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'name' => array('type' => 'string', 'null' => false, 'length' => 50),
+			),
+			'other_table' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'name' => array('type' => 'string', 'null' => false, 'length' => 50),
+			)
+		));
+		$schema2 =& new CakeSchema(array(
+			'name' => 'AlterTest1',
+			'connection' => 'test_suite',
+			'altertest' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'field_two' => array('type' => 'string', 'null' => false, 'length' => 50),
+			),
+			'other_table' => array(
+				'id' => array('type' => 'integer', 'null' => false, 'default' => 0),
+				'field_two' => array('type' => 'string', 'null' => false, 'length' => 50),
+			)
+		));
+		$result = $this->db->alterSchema($schema2->compare($schema1));
+		$this->assertEqual(2, substr_count($result, 'field_two'), 'Too many fields');
+		$this->assertFalse(strpos(';ALTER', $result), 'Too many semi colons');
+	}
 }