Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug(SQLite3): Forge::dropColumn() seems to always return false #8849

Open
mkuettel opened this issue May 2, 2024 · 1 comment · May be fixed by #8931
Open

Bug(SQLite3): Forge::dropColumn() seems to always return false #8849

mkuettel opened this issue May 2, 2024 · 1 comment · May be fixed by #8931
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer

Comments

@mkuettel
Copy link

mkuettel commented May 2, 2024

PHP Version

8.3

CodeIgniter4 Version

4.5.1

CodeIgniter4 Installation Method

Composer (as dependency to an existing project)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

cli

Database

SQLite3

What happened?

I was checking whether dropColumn of forge returned false when migrating down. Which always seemed to be the case.
When checking the implementation I noticed that Forge expects from the driver that _alterTable returns a SQL statement to be executed, but in this case SQLite3 will return an empty string, because it runs multiple statements in the background, creating a new temporary table without the specified columns, moving data over and renaming afterwards.

Steps to Reproduce

<?php

declare(strict_types=1);

namespace App\Database\Migrations;

use CodeIgniter\Database\Exceptions\DatabaseException;
use CodeIgniter\Database\Migration;

class AddAColumnToExistingTable extends Migration
{
    protected string $table = 'existing_table';

    public function up(): void
    {
        $this->forge->addColumn($this->table, [
            'column'   => ['type' => 'varchar', 'null' => false],
        ]) or throw new DatabaseException("Could not add columns to table {$this->table}: " . var_export($this->db->error(), true));
    }

    public function down(): void
    {
        $this->forge->dropColumn($this->table, 'column')
            or throw new DatabaseException("Could drop column from table {$this->table}: " . var_export($this->db->error(), true));
    }
}

Expected Output

None, but the DatabaseException is thrown in the down() method, telling me that no error occured.
But dropColumn() returned false, because the statement is empty.

Anything else?

No response

@mkuettel mkuettel added the bug Verified issues on the current code behavior or pull requests that will fix them label May 2, 2024
@mkuettel mkuettel changed the title Bug(SQLite3): forge->dropColumn seems to always return false. Bug(SQLite3): Forge::dropColumn() seems to always return false May 2, 2024
@kenjis kenjis added the database Issues or pull requests that affect the database layer label May 2, 2024
@kenjis
Copy link
Member

kenjis commented May 2, 2024

I confirmed the bug.

--- a/tests/system/Database/Live/ForgeTest.php
+++ b/tests/system/Database/Live/ForgeTest.php
@@ -1317,7 +1317,9 @@ final class ForgeTest extends CIUnitTestCase
 
         $this->assertTrue($this->db->fieldExists('name', 'forge_test_two'));
 
-        $this->forge->dropColumn('forge_test_two', 'name');
+        $return = $this->forge->dropColumn('forge_test_two', 'name');
+
+        $this->assertTrue($return);
 
         $this->db->resetDataCache();
 
1) CodeIgniter\Database\Live\ForgeTest::testDropColumn
Failed asserting that false is true.

@ping-yee ping-yee linked a pull request Jun 1, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants