Fixtures force autoincrement on ID column

[thinkingmedia]

This is a (multiple allowed):

• bug

• enhancement

• feature-discussion (RFC)

• CakePHP Version: 3.3.11

• Platform and Target: Apache2, MySQL, PHP 7.1, Bash on Windows 10

What you did

Created a fixture that imports the schema.

What happened

Here is the production table named ratings. Notice that by design it does not have an auto-increment on the primary ID column. This table stores film ratings and I use the numeric age as the primary key.

CREATE TABLE `ratings` (
  `id` int(11) NOT NULL,
  `key` char(5) COLLATE utf8mb4_unicode_ci NOT NULL,
  `title` varchar(80) COLLATE utf8mb4_unicode_ci NOT NULL,
  `created` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  `updated` datetime NOT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `id` (`id`),
  KEY `created` (`created`),
  KEY `updated` (`updated`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci

When I created a fixture for ratings and used the schema import feature. The fixture manager created a table with an auto-increment ID. This creates problems for my code.

Here's the fixture table that was created.

CREATE TABLE `ratings` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `key` char(5) COLLATE utf8mb4_unicode_ci NOT NULL,
  `title` varchar(80) COLLATE utf8mb4_unicode_ci NOT NULL,
  `created` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  `updated` datetime NOT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `id` (`id`),
  KEY `created` (`created`),
  KEY `updated` (`updated`)
) ENGINE=InnoDB AUTO_INCREMENT=20 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci

Ignore the AUTO_INCREMENT=20 as I dumped the create statement as the tests ran.

What you expected to happen

Should not create an auto-increment column unless the source table has that attribute.

[markstory]

It looks like this is happening during schema generation, after the reflection is complete. The MySqlSchema class does that here. I'm pretty sure this is a backwards compatibility shim that was intended to help migrating from 2.x easier.

Old fixtures would never have the autoIncrement key set in their $fields as that key didn't exist in 2.x.

[thinkingmedia]

I added this to my fixture, and it outputs false.

    public function __construct()
    {
        parent::__construct();
        /* @var Connection $conn */
        $conn = ConnectionManager::get('test', false);
        var_dump($this->_schema->hasAutoincrement());
    }

So the schema object is being built correctly. The PHP debugger shows that autoIncrement is set to NULL for the ID column.

So I don't think this is a problem with the MySqlSchema class, but I could be wrong. I need to look closer at it.

[thinkingmedia]

@markstory oh you're correct!

This is the part here.

        $addAutoIncrement = (
            [$name] == (array)$table->primaryKey() &&
            !$table->hasAutoIncrement()
        );

This forces an auto-increment if there is a primary key and no auto-increment in the schema.

[thinkingmedia]

Do you think we could have this piece of logic removed in 3.4? I looked again at the code this morning, and there is no work around for it. Either we remove it or we add some way of disabling it temporary.

In order to get my tests to work. I did this (for anyone else having the issue).

/**
 * RatingsFixture
 */
class RatingsFixture extends GemsFixture
{
    /**
     * @param ConnectionInterface|Connection $db
     * @return bool
     */
    public function create(ConnectionInterface $db)
    {
        $result = parent::create($db);
        if ($result) {
            $db->transactional(function () use ($db) {
                $db->disableConstraints(function () use ($db) {
                    $db->execute("ALTER TABLE `test_ahtag`.`ratings` CHANGE COLUMN `id` `id` INT(11) NOT NULL");
                });
            });
        }
        return $result;
    }
}

[markstory]

@thinkingmedia We could try removing that logic. Not generating the same primary key as the source table had/fixture defined is under the umbrella of defect. The solution for app developers would be to add 'autoIncrement' => true to their fixture definitions.

[thinkingmedia]

@markstory I'm good now with my hack. We could mark this for 3.4 as a deprecated fix for 2.0 migrations.

[thinkingmedia]

@markstory I was tempted to open a PR today for this. Do you think it should be changed or should we just leave it as is for now.

[markstory]

I think it would be good if schema reflection resulted in the same table being generated by the fixtures. This could be something that goes into 3.5.0. I think we'd need to document this behavior change to folks who might be relying on it.