Merge pull request #10356 from sebastienbarre/duplicate_primary

fix duplicate primary keys for tables without models

lib/Cake/Model/CakeSchema.php

@@ -613,9 +613,17 @@ class CakeSchema extends CakeObject {
 		$db = $Obj->getDataSource();
 		$fields = $Obj->schema(true);
 
+		$hasPrimaryAlready = false;
+		foreach ($fields as $value) {
+			if (isset($value['key']) && $value['key'] === 'primary') {
+				$hasPrimaryAlready = true;
+				break;
+			}
+		}
+
 		$columns = array();
 		foreach ($fields as $name => $value) {
-			if ($Obj->primaryKey === $name) {
+			if ($Obj->primaryKey === $name && !$hasPrimaryAlready && !isset($value['key'])) {
 				$value['key'] = 'primary';
 			}
 			if (!isset($db->columns[$value['type']])) {

lib/Cake/Model/Datasource/Database/Postgres.php

@@ -198,14 +198,26 @@ class Postgres extends DboSource {
 		$fields = parent::describe($table);
 		$this->_sequenceMap[$table] = array();
 		$cols = null;
+		$hasPrimary = false;
 
 		if ($fields === null) {
 			$cols = $this->_execute(
-				"SELECT DISTINCT table_schema AS schema, column_name AS name, data_type AS type, is_nullable AS null,
+				'SELECT DISTINCT table_schema AS schema,
-					column_default AS default, ordinal_position AS position, character_maximum_length AS char_length,
+					column_name AS name,
-					character_octet_length AS oct_length FROM information_schema.columns
+					data_type AS type,
-				WHERE table_name = ? AND table_schema = ?  ORDER BY position",
+					is_nullable AS null,
-				array($table, $this->config['schema'])
+					column_default AS default,
+					ordinal_position AS position,
+					character_maximum_length AS char_length,
+					character_octet_length AS oct_length,
+					pg_get_serial_sequence(attr.attrelid::regclass::text, attr.attname) IS NOT NULL AS has_serial
+				FROM information_schema.columns c
+				INNER JOIN pg_catalog.pg_namespace ns ON (ns.nspname = table_schema)
+				INNER JOIN pg_catalog.pg_class cl ON (cl.relnamespace = ns.oid AND cl.relname = table_name)
+				LEFT JOIN pg_catalog.pg_attribute attr ON (cl.oid = attr.attrelid AND column_name = attr.attname)
+				WHERE table_name = ? AND table_schema = ? AND table_catalog = ?
+				ORDER BY ordinal_position',
+				array($table, $this->config['schema'], $this->config['database'])
 			);
 
 			// @codingStandardsIgnoreStart
@@ -238,17 +250,25 @@ class Postgres extends DboSource {
 						"$1",
 						preg_replace('/::.*/', '', $c->default)
 					),
-					'length' => $length
+					'length' => $length,
 				);
-				if ($model instanceof Model) {
+
-					if ($c->name === $model->primaryKey) {
+				// Serial columns are primary integer keys
-						$fields[$c->name]['key'] = 'primary';
+				if ($c->has_serial) {
-						if (
+					$fields[$c->name]['key'] = 'primary';
-							$fields[$c->name]['type'] !== 'string' &&
+					$fields[$c->name]['length'] = 11;
-							$fields[$c->name]['type'] !== 'uuid'
+					$hasPrimary = true;
-						) {
+				}
-							$fields[$c->name]['length'] = 11;
+				if ($hasPrimary === false &&
-						}
+					$model instanceof Model &&
+					$c->name === $model->primaryKey
+				) {
+					$fields[$c->name]['key'] = 'primary';
+					if (
+						$fields[$c->name]['type'] !== 'string' &&
+						$fields[$c->name]['type'] !== 'uuid'
+					) {
+						$fields[$c->name]['length'] = 11;
 					}
 				}
 				if (

lib/Cake/Test/Case/Model/CakeSchemaTest.php

@@ -363,6 +363,39 @@ class SchemaCrossDatabaseFixture extends CakeTestFixture {
 	);
 }
 
+/**
+ * NonConventionalPrimaryKeyFixture class
+ *
+ * @package       Cake.Test.Case.Model
+ */
+class NonConventionalPrimaryKeyFixture extends CakeTestFixture {
+
+/**
+ * name property
+ *
+ * @var string
+ */
+	public $name = 'NonConventional';
+
+/**
+ * table property
+ *
+ * @var string
+ */
+	public $table = 'non_conventional';
+
+/**
+ * fields property
+ *
+ * @var array
+ */
+	public $fields = array(
+		'version_id' => array('type' => 'integer', 'key' => 'primary'),
+		'id' => array('type' => 'integer'),
+		'name' => 'string'
+	);
+}
+
 /**
  * SchemaPrefixAuthUser class
  *
@@ -649,6 +682,33 @@ class CakeSchemaTest extends CakeTestCase {
 		$fixture->drop($db);
 	}
 
+/**
+ * testSchemaRead method when a primary key is on a non-conventional column
+ *
+ * @return void
+ */
+	public function testSchemaReadWithNonConventionalPrimaryKey() {
+		$db = ConnectionManager::getDataSource('test');
+		$fixture = new NonConventionalPrimaryKeyFixture();
+		$fixture->create($db);
+
+		$read = $this->Schema->read(array(
+			'connection' => 'test',
+			'name' => 'TestApp',
+			'models' => false
+		));
+		$fixture->drop($db);
+
+		$this->assertArrayHasKey('non_conventional', $read['tables']);
+		$versionIdHasKey = isset($read['tables']['non_conventional']['version_id']['key']);
+		$this->assertTrue($versionIdHasKey, 'version_id key should be set');
+		$versionIdKeyIsPrimary = $read['tables']['non_conventional']['version_id']['key'] === 'primary';
+		$this->assertTrue($versionIdKeyIsPrimary, 'version_id key should be primary');
+
+		$idHasKey = isset($read['tables']['non_conventional']['id']['key']);
+		$this->assertFalse($idHasKey, 'id key should not be set');
+	}
+
 /**
  * test that tables are generated correctly
  *