Framework: DropColumnIfExists

[SaifurRahmanMohsin]

drop has a dropIfExists. But why does dropColumn not have a dropColumnIfExists function?

I feel that the method should exist as when I tried some migrations for pluggable components which has their own migrations, I faced an issue in this scenario:

1. Plugin B modifies migration of Plugin A to add column user_id.
2. Plugin A is reset (rollback and upgrade), now when Plugin B is reset it throws an error:

[PDOException]
SQLSTATE[42000]: Syntax error or access violation: 1091 Can't DROP 'user_id'; check that column/key exists

I ended up changing

public function down()
{
    if (Schema::hasColumn(‘users', 'user_id'))
    {
        $table->dropColumn(['user_id']);
    }
}

to:

public function down()
{
    if (Schema::hasColumn(‘users', 'user_id'))
    {
        Schema::table('users', function($table) {
            $table->dropColumn(['user_id']);
        });
    }
}

I feel it would be simpler to just have a dropColumnIfExists. Makes it consistent with the drop - dropIfExists pair as well.

[srmklive]

Here the reason why i gave a thumbs down reaction. Because dropColumnIfExists doesn't have a direct SQL translation. Take the example of the commands you listed:

drop (DROP TABLE foo;)
dropIfExists (DROP TABLE IF EXISTS foo;)
dropColumn (ALTER TABLE foo DROP COLUMN bar;)

So by this pattern dropColumnIfExists should return ALTER TABLE foo DROP COLUMN IF EXISTS bar; which is invalid SQL.

Here is how this code block works:

    Schema::table('users', function($table) {
        $table->dropColumn(['user_id']);
    });

• First Schema/Builder.php is loaded. This deals with the database itself.
• For callback Schema/Blueprint.php is loaded. This deals with the table.

What you are suggesting is setting a $builder property inside a Blueprint.php file to access Builder.php methods, which means every time you call any methods through Schema you will always set the $builder property. $builder will always remain redundant & of no use unless you specifically call dropColumnIfExists in Blueprint.php.

I can't think of any reason to have a dropColumnIfExists

[miscbits]

I agree above with @srmklive that the method doesn't make sense just because it does not translate to SQL. I do however think there is something to the issue that should be addressed.

Perhaps for now though the real issue is a package assuming it can safely modify an existing table. For the sake of non-collision, either the developer of a project needs to rollback migrations in the order they migrated them, or the developer has to reject any package that tries to modify an existing table. Instead, a package should create a 1-1 relationship with an existing table and a new table generated with a migration in the package.

[SaifurRahmanMohsin]

If Eloquent is just a translation library might as well use DROP TABLE foo;, DROP TABLE IF EXISTS foo;, etc. There would be no point for the Eloquent ORM system. I see the whole point of this is to bring in schema building and migration scripts and to abstract various DBs SQL statements to a more standardised format making it simpler for developers.

My issue was just pointing out the flaw in the system that pluggable components break down because of this (do see the Plugin A, B sample scenario I provided). Like @miscbits suggested, perhaps this becomes the developer’s headache now since Eloquent system fails to cover this flaw and collisions do occur.

[srmklive]

@SaifurRahmanMohsin kindly look at my last reply as why i think i don't see any compelling reasons for inclusion of such method in the framework. However if you believe that it is a issue, instead of discussing it back & forth, why don't you simply generate a PR for inclusion into the framework.

[themsaid]

Closing this since it's not a bug report, but please feel free to open a PR in case you're still willing to propose such functionality.