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

Schema builder: Dropping a column with a default value constraint #4402

Closed
soxyl opened this issue May 13, 2014 · 22 comments
Closed

Schema builder: Dropping a column with a default value constraint #4402

soxyl opened this issue May 13, 2014 · 22 comments

Comments

@soxyl
Copy link

soxyl commented May 13, 2014

I'm using Laravel 4.1 with the sqlsrv database driver and I'm running in a problem with the default value constraints in the migration scripts.

When I add a column to a table with a default value constraint the name of the constraint is randomly generated (e.g. DF__foo__bar__322242A7). The problem occurs now in the rollback method because it is not possible to drop the column as long as the default constraint exists and it is not possible to drop the constraint because its name is randomly generated by the sql server.

I think that it would be good if the Laravel framework would generate the names of default value constraints like id does it for the primary, the foreign and the unique constraints.

Schema::table('foo', function($table)
{
     $table->boolean('bar')->default(false);
});
Schema::table('foo', function($table)
{
    $table->dropColumn('bar'); // Don't works!
});

// [Illuminate\Database\QueryException]
// SQLSTATE[42000]: [Microsoft][SQL Server Native Client 11.0][SQL Server]The object 'DF__foo__bar__322242A7' is dependent on column 'bar'. (SQL: alter table "foo" drop column "bar")
@taylorotwell
Copy link
Member

I'm not that familiar with SQL Server. If you want to submit a pull request to fix this behavior please feel free.

@joestrong
Copy link

I've stumbled upon the same issue today. Creating a column with a default value on SQL Server creates a constraint that stops the ability to drop the column.

@cowabunga1
Copy link

Column name change also generates an error...

@sherwinmartin
Copy link

any update on this? it's happening on laravel 5.1 and mssql. it's fine if you use Schema::drop() but the error occurs when you use $table->dropColumn().

@rafis
Copy link
Contributor

rafis commented Sep 22, 2016

Temporary solution:

    $tableName = 'myTable';
    if ('sqlsrv' == DB::connection()->getDriverName()) {
        $defaultContraint = DB::selectOne("SELECT OBJECT_NAME([default_object_id]) AS name FROM SYS.COLUMNS WHERE [object_id] = OBJECT_ID('[dbo].[$tableName]') AND [name] = 'myField'");
        DB::statement("ALTER TABLE [dbo].[$tableName] DROP CONSTRAINT $defaultContraint->name");
    }

    Schema::table($tableName, function (Blueprint $table) {
        $table->dropColumn('myField');
    });

But more complete solution is to assign explicit name to default contraint as described in #8827 (this requires developing a pull request - anyone please?).

@FrittenKeeZ
Copy link

FrittenKeeZ commented Jul 12, 2017

I just ran into this problem as well. Why was this issue closed?
Otherwise a notice for the default modifier, that SQL Server can't rollback automatically, would be nice.

Edit: I just did some research and found this piece of code which fetches all constraints for a list of columns.

SELECT OBJECT_NAME([default_object_id]) 
FROM SYS.COLUMNS
WHERE [object_id] = OBJECT_ID('[tableSchema].[tableSchema]') AND [name] IN ('column1', 'column2', 'column3');

This can be used in conjunction with Illuminate\Database\Schema\Grammars\SqlServerGrammar::compileDropColumn() to drop the constraints before the columns.

$sqlConstraints = count($constraints) ? ' drop constraint ' . implode(', ', $constraints) . ',' : '';
return sprintf('alter table %s%s drop column %s',
    $this->wrapTable($blueprint),
    $sqlConstraints,
    implode(', ', $columns)
);

It just doesn't seem like the right place to put it, but I don't know where else it could be placed.
@taylorotwell what are your thoughts on this?

@oranges13
Copy link

Just another note to say I just had this issue with Sql Server and the solution in @rafis comment fixed it, but it does seem somewhat janky.

@phattarachai
Copy link

Hi, today I run into this problem as well, I wish to have this functionality work from Laravel just like with other database driver.

@zrm-eeifler
Copy link

This is sadly still an issue, and I hope it will be addressed in the future. Illuminate\Foundation\Testing\RefreshDatabase for testing will not work either, of course.

@FrittenKeeZ
Copy link

FrittenKeeZ commented Apr 4, 2019

Inspired by @holystix solution in their referenced fix, I made a Blueprint macro to drop the default constraint without polluting the migration files.

use Illuminate\Support\Facades\Schema;
use Illuminate\Database\Schema\Blueprint;

Blueprint::macro('dropDefaultConstraint', function (string $column) {
    $connection = Schema::getConnection();
    if ($connection->getDriverName() === 'sqlsrv') {
        $query = sprintf(
            'SELECT OBJECT_NAME([default_object_id]) AS name ' .
            'FROM SYS.COLUMNS ' .
            "WHERE [object_id] = OBJECT_ID('[dbo].[%s]') AND [name] = '%s'",
            $this->getTable(),
            $column
        );
        $result = $connection->selectOne($query);
        $constraint = new \Doctrine\DBAL\Schema\Index($result->name, [$column]);
        $connection->getDoctrineSchemaManager()->dropConstraint($constraint, $this->getTable());
    }
});

Just put it in AppServiceProvider::boot() and call it like any other drop statement $table->dropDefaultConstraint('active');

@dominik-stypula-polcode

Just put it in AppServiceProvider::boot() and call it like any other drop statement $table->dropDefaultContraint('active');

This macro throws an exception.

   PDOException  : SQLSTATE[IM001]: Driver does not support this function: driver does not support that attribute

  at /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:52
    48|      * {@inheritdoc}
    49|      */
    50|     public function getServerVersion()
    51|     {
  > 52|         return PDO::getAttribute(PDO::ATTR_SERVER_VERSION);
    53|     }
    54| 
    55|     /**
    56|      * {@inheritdoc}

  Exception trace:

  1   PDO::getAttribute()
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:52

  2   Doctrine\DBAL\Driver\PDOConnection::getServerVersion()
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:475

  3   Doctrine\DBAL\Connection::getServerVersion()
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:461

  4   Doctrine\DBAL\Connection::getDatabasePlatformVersion()
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:388

  5   Doctrine\DBAL\Connection::detectDatabasePlatform()
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:330

  6   Doctrine\DBAL\Connection::getDatabasePlatform()
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php:53

  7   Doctrine\DBAL\Schema\AbstractSchemaManager::__construct(Object(Doctrine\DBAL\Connection))
      /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractSQLServerDriver.php:79

  8   Doctrine\DBAL\Driver\AbstractSQLServerDriver::getSchemaManager(Object(Doctrine\DBAL\Connection))
      /var/www/html/vendor/laravel/framework/src/Illuminate/Database/Connection.php:887

  9   Illuminate\Database\Connection::getDoctrineSchemaManager()
      /var/www/html/app/MailRedirector/Helpers/MSSqlDropDefaultDFKeyMacro.php:32

  10  Illuminate\Database\Schema\Blueprint::App\MailRedirector\Helpers\{closure}("buyer_id")
      /var/www/html/vendor/laravel/framework/src/Illuminate/Support/Traits/Macroable.php:108

  11  call_user_func_array(Object(Closure))
      /var/www/html/vendor/laravel/framework/src/Illuminate/Support/Traits/Macroable.php:108

  12  Illuminate\Database\Schema\Blueprint::__call("dropDefaultConstraint")
      /var/www/html/database/migrations/2019_12_11_114257_additional_fields_for_redirect_history_table.php:76

@dominik-stypula-polcode

But good news is i managed to fix your code. This version works for me.
WARNING! I fixed misspelled 'dropDefaultConstraint', so make sure that you change invocations if you copied it from previous version, and I added $databaseConnectionName that you get from your database config file.

$databaseConnectionName = 'my-mssql-connection';
Blueprint::macro('dropDefaultConstraint', function (string $column) use ($databaseConnectionName) {
            $connection = Schema::connection($databaseConnectionName)->getConnection();
            if ($connection->getDriverName() === 'sqlsrv') {
                $query = sprintf(
                    'SELECT OBJECT_NAME([default_object_id]) AS name ' .
                    'FROM SYS.COLUMNS ' .
                    "WHERE [object_id] = OBJECT_ID('[dbo].[%s]') AND [name] = '%s'",
                    $this->getTable(),
                    $column
                );
                $result = $connection->selectOne($query);
                $constraint = new Index($result->name, [$column]);
                $tableName = $this->getTable();
                $keyName = $constraint->getName();
                $sql = "ALTER TABLE  {$tableName} DROP CONSTRAINT {$keyName}";
                $connection->statement($sql);
                //$connection->getDoctrineSchemaManager()->dropConstraint($constraint, $this->getTable());
            }
        });

@joelharkes
Copy link
Contributor

Why is this not in the default? should be checked when drop column happens in laravel.

This issue shouldn't be closed, its still an issue to date.

@joelharkes
Copy link
Contributor

Here you have a better fix, it works as long as you only use one drop column statement with array notation: $table->dropColumn(['column_a','column_b']); since @sql can only be declared once in T-SQL otherwise it will probably throw errors.

class SqlServerGrammarFixed extends SqlServerGrammar
{
    public function compileDropColumn(Blueprint $blueprint, Fluent $command)
    {
        echo 'testsetst';
        $dropConstraintsSql = $this->compileDropDefaultConstraint($blueprint, $command);
        $base = parent::compileDropColumn($blueprint, $command);

        Log::debug($dropConstraintsSql);

        return $dropConstraintsSql . '; ' . $base;
    }

    protected function compileDropDefaultConstraint(Blueprint $blueprint, Fluent $command)
    {
        $sql = "DECLARE @sql NVARCHAR(MAX) = '';";
        $tableName = $blueprint->getTable();
        $columns = $this->wrapArray($command->columns);
        Log::info($columns);
        $columnSql = "'" . implode("','", $command->columns) . "'";
        $sql .= "SELECT @sql += 'ALTER TABLE [dbo].[$tableName] DROP CONSTRAINT ' + OBJECT_NAME([default_object_id]) + ';' ";
        $sql .= 'FROM SYS.COLUMNS ';
        $sql .= "WHERE [object_id] = OBJECT_ID('[dbo].[$tableName]') AND [name] in ($columnSql);";
        $sql .= 'EXEC(@sql)';

        return $sql;
    }
}

class SqlServerConnectionFixed extends SqlServerConnection
{
    protected function getDefaultSchemaGrammar()
    {
        return $this->withTablePrefix(new SqlServerGrammarFixed());
    }
}

	

IN DatabaseServiceProvider.php

Connection::resolverFor('sqlsrv', function ($connection, $database, $prefix, $config) {
            return new SqlServerConnectionFixed($connection, $database, $prefix, $config);
        });

@yaroslavmo
Copy link
Contributor

I am confused. I am using Laravel 6.0.4 and facing this 6 years old issue. Any updates when it will be fixed?

@oranges13
Copy link

It looks like there is a PR proposed in #31229

@joelharkes
Copy link
Contributor

joelharkes commented Jan 27, 2020

@yaroslavmo see my comment for quick fix for now, until hopefully they will actually approve one of my PR's (upto now all my Pr's have been instantly denied/closed).

@yaroslavmo
Copy link
Contributor

Thank you. I used the workaround @rafis suggested (only on the local machine). I will wait till they review your PR. I strongly believe this kind of thing should be the same for all four DB connectors Laravel supports and should be fixed on the framework level. For now, it is hard to use sqlsrv as a database.

P.S. if they deny your PR, I will have to use your fix tho 😃

@joelharkes
Copy link
Contributor

It has been merged. will probably be release in next laravel version

@yaroslavmo
Copy link
Contributor

Hi @joelharkes . Your PR did not work for one of my migrations.

<?php

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Migrations\Migration;

class UpgradeCors extends Migration
{
    /**
     * Run the migrations.
     *
     * @return void
     */
    public function up()
    {
        Schema::table('cors_config', function (Blueprint $table) {
            $table->string('description')->nullable()->after('path');
            $table->text('exposed_header')->nullable()->after('header');
            $table->boolean('supports_credentials')->default(0)->after('max_age');
        });
    }

    /**
     * Reverse the migrations.
     *
     * @return void
     */
    public function down()
    {
        Schema::table('cors_config', function (Blueprint $table) {
            $table->dropColumn(['description', 'exposed_header', 'supports_credentials']);
        });
    }
}

It gives this error:

[Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The object 'DF__cors_conf__suppo__44428990' is dependent on column 'supports_credentials'. (SQL: DECLARE @SQL NVARCHAR(MAX) = ''
;SELECT @SQL += 'ALTER TABLE [dbo].[cors_config] DROP CONSTRAINT ' + OBJECT_NAME([default_object_id]) + ';' FROM SYS.COLUMNS WHERE [object_id] = OBJECT_ID('[dbo].[cors_config]') AND [name] in ('descr
iption','exposed_header','supports_credentials');EXEC(@SQL);alter table "cors_config" drop column "description", "exposed_header", "supports_credentials")

I tried to debug it myself but I am not that familiar with SQL Server and it is super unclear to me why created default object is called 'DF__cors_conf__suppo__44428990' and not 'DF__cors_conf__supports_credentials__44428990'

@yaroslavmo
Copy link
Contributor

yaroslavmo commented Feb 3, 2020

This is the SQL query generated by your PR:

DECLARE @sql NVARCHAR(MAX) = '';SELECT @sql += 'ALTER TABLE [dbo].[cors_config] DROP
    CONSTRAINT ' + OBJECT_NAME([default_object_id]) + ';' FROM SYS.COLUMNS WHERE
    [object_id] = OBJECT_ID('[dbo][cors_config]') AND [name] in ('description', 'exposed_header',
    'supports_credentials'); EXEC(@sql); 
    alter table 'cors_config' drop column 'description', 'exposed_header', 'supports_creadentials' 

Which fails with the same error when I run it in SQL Server Management Studio.
While this query will work just fine:

DECLARE @sql NVARCHAR(MAX) = '';SELECT @sql += 'ALTER TABLE [dbo].[cors_config] DROP
    CONSTRAINT ' + OBJECT_NAME([default_object_id]) + ';' FROM SYS.COLUMNS WHERE
    [object_id] = OBJECT_ID('[dbo][cors_config]') AND [name] = 'supports_credentials'; EXEC(@sql); 
    alter table 'cors_config' drop column 'description', 'exposed_header', 'supports_creadentials' 

This will also work if I just add "not equals":
AND [name] in('description', 'exposed_header', 'supports_credentials') AND [default_object_id] <> 0; EXEC(@sql);
So I believe something wrong with IN statement. @joelharkes What do you think?

@joelharkes
Copy link
Contributor

Weird let me check if I can reproduce it tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests