codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.39k stars 1.9k forks source link

Bug: SQLite DB Duplicate Indexes #6205

Closed MGatner closed 2 years ago

MGatner commented 2 years ago

PHP Version

7.4

CodeIgniter4 Version

4.2.1

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

cli

Database

SQLite3

What happened?

I have the following migrations:

  1. Create table
  2. Modify that table add column
  3. Modify that table drop column

The down() methods do the reverse, so for example 3) looks like:


    public function up(): void
    {
        $this->forge->dropColumn('actions', [
            'description',
            'summary',
            'category',
            'icon',
            'role',
            'class',
        ]);
    }

    public function down(): void
    {
        $this->forge->addColumn('actions', [
            'class'       => ['type' => 'varchar', 'constraint' => 63, 'null' => true],
            'role'        => ['type' => 'varchar', 'constraint' => 63, 'default' => ''],
            'icon'        => ['type' => 'varchar', 'constraint' => 63, 'default' => ''],
            'category'    => ['type' => 'varchar', 'constraint' => 63, 'default' => ''],
            'summary'     => ['type' => 'varchar', 'constraint' => 255],
            'description' => ['type' => 'text', 'constraint' => 255],
        ]);
    }

When running test where the database is migrated up and down multiple times the dropColumn() command in the last migration is duplicating indexes (name in this case):

┌──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ $sqls                                                                                                                                                                                                │
└──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
array (5) [
    0 => string (56) "CREATE INDEX `db_actions_name` ON `db_actions` (`name`);"
    1 => string (54) "CREATE INDEX `db_actions_uid` ON `db_actions` (`uid`);"
    2 => string (56) "CREATE INDEX `db_actions_name` ON `db_actions` (`name`);"
    3 => string (77) "CREATE INDEX `db_actions_deleted_at_id` ON `db_actions` (`deleted_at`, `id`);"
    4 => string (68) "CREATE INDEX `db_actions_created_at` ON `db_actions` (`created_at`);"
]
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Called from <ROOT>/vendor/codeigniter4/framework/system/Database/Forge.php:521 [d()]

Steps to Reproduce

This is on the following repo: https://github.com/tattersoftware/codeigniter4-workflows

I'm trying to make a new migration (as pasted above).

Expected Output

I'm not sure if this is specific to the weird way SQLite3 handles indexes, but however this method is getting triggered it needs to be ensuring that the keys are unique.

Anything else?

No response

MGatner commented 2 years ago

Found the root of the issue. The original migration defines a compound index (category, name). When dropColumn() is called on "category" the index persists in $keys:

┌──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ $this->keys                                                                                                                                                                                          │
└──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
array (5) [
    0 => array (1) [
        0 => string (4) "name"
    ]
    1 => array (1) [
        0 => string (3) "uid"
    ]
    2 => array (2) [
        0 => string (8) "category"
        1 => string (4) "name"
    ]
    3 => array (2) [
        0 => string (10) "deleted_at"
        1 => string (2) "id"
    ]
    4 => array (1) [
        0 => string (10) "created_at"
    ]
]
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Called from <ROOT>/vendor/codeigniter4/framework/system/Database/Forge.php:520 [d()]

... but since the column is no longer present Forge somehow reduces this to a simple index: CREATE INDEXdb_actions_nameONdb_actions(name);. Since "name" already has an index this fails.

I'm not sure what the expected behavior should be here, but my hunch is that "consolidating" the compound index into a simple one is a mistake.

MGatner commented 2 years ago

I was able to work around this by dropping the index before dropping the columns:

$this->forge->dropKey('actions', 'actions_category_name');