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

[Proposal] Add Schema::hasIndex() method #3253

Closed
billmn opened this issue Jan 15, 2014 · 31 comments
Closed

[Proposal] Add Schema::hasIndex() method #3253

billmn opened this issue Jan 15, 2014 · 31 comments

Comments

@billmn
Copy link
Contributor

billmn commented Jan 15, 2014

What about add a method to the Schema class to check if an index exists?
It can be useful in migrations to avoid possible errors on adding/dropping indexes

I know a method on MySQL but I haven't experience with other DB engines.
I've found this informations to make this:

Thoughts?

@bencorlett
Copy link
Contributor

Hey mate,

It would be the first example of this sort of logic (e.g. querying the database) inside the Schema class, therefore it's probably better suited in the DB Query classes, though, it'd be the first schema-related thing there, so it's tricky.

Question, when would you not know if you had an index? If each migration's up/down methods null each other out, there shouldn't be a discrepancy, right?

@billmn
Copy link
Contributor Author

billmn commented Jan 16, 2014

Hey Ben,
yep ... there shouldn't be a discrepancy migration up/down ... but is possibile (a mistake, an oversight).

To avoid to stop a migration ( also avoiding all the problems that this thing can cause ) I think that a "hasIndex" or a "dropIndexIfExists" method can be helpful

@taylorotwell
Copy link
Member

Doctrine can probably handle this. The DBAL is accessible via the DB connection.

@guiwoda
Copy link
Contributor

guiwoda commented Aug 12, 2014

@taylorotwell Sorry to bump this up, but it's unclear to me how you suggest to access the DBAL instance inside a migration.
I'd rather do something like this:

Schema::table('users', function(Blueprint $table){
    // ...
    if (! $table->hasIndex('remember_token'))
        $table->index('remember_token');

    // OR, altogether...
    $table->indexIfNotExists('remember_token');
});

Than something like this:

Schema::table('users', function(Blueprint $table){
    // ...
    if (! DB::connection()->getDoctrineSchemaBuilder()->hasIndex('users_remember_token_index'))
        $table->index('remember_token');
});

Where, in the second example, I'm hard-coding laravel's way of doing indexes as well as explicitly bringing up Doctrine's dependency on my code.
I feel that this kind of stuff makes Laravel what it is: a nice set of semantic Proxys over complex models / vendors.

@guiwoda
Copy link
Contributor

guiwoda commented Aug 12, 2014

I'll have to add to this, I really don't find the way to do it with Doctrine.
I've never used DBAL, and haven't gone too far into Laravel's code to understand when it does.

@taylorotwell as you said it's easily done with DBAL, can you post a small snippet of code of how it could be done?

UPDATE
This is how it can be done:

$conn = Schema::getConnection();
$dbSchemaManager = $conn->getDoctrineSchemaManager();
$doctrineTable = $dbSchemaManager->listTableDetails('users');

// alter table "users" add constraint users_email_unique unique ("email")
if (! $doctrineTable->hasIndex('users_email_unique'))
{
    $table->unique('email');
}

@ee0pdt
Copy link

ee0pdt commented May 5, 2015

Why is this closed? It's a perfectly valid request and necessary for avoiding errors on complicated migration sets.

But thanks @guiwoda for showing how to do this with DBAL

@mikeher
Copy link

mikeher commented Nov 13, 2015

@guiwoda Awesome, I'll try to work this into my migrations. Thanks!

I'm currently facing a problem where adding a unique composite index in a later migration, removes an index on one of the attributes in my new index, which in turn leads to an exception when rolling back, because an index is needed for the fkey to work.

My solution: To manually add the old index before deleting the unique index. However, the new index does not get deleted when migrating the new unique key once more, even though it is identical to the one originally created by the foreign() method. Because of this I am now checking whether such an index exists before adding it in my down() method.

@thiagomata
Copy link

+1

@leek
Copy link

leek commented Dec 14, 2015

It sucks that this was closed considering we already have hasTable(), hasColumn(), etc. This seems like a logical addition IMO.

@cyrrill
Copy link

cyrrill commented Dec 15, 2015

I also would like to request the reopening this issue. Having the ability to check for an index seems like a very natural function for a Schema class.

@DennisXH
Copy link

+1 thanks @guiwoda

1 similar comment
@martynling
Copy link

+1 thanks @guiwoda

@PiNotEqual3
Copy link

+1

@teamhoodin
Copy link

+1 thanks @guiwoda

@golinmarq
Copy link

This shouldn't be closed

@salarmehr
Copy link

They look for PR not just suggestion.

@itguy614
Copy link

I have run into this often enough that I'm putting together a PR for this.

@itguy614
Copy link

This is in PR #14975

@xiaohigh
Copy link

+1

@guiwoda
Copy link
Contributor

guiwoda commented Sep 22, 2016

I appreciate the +1s, but can we please close this conversation?
It's over two years old.

@marlocorridor
Copy link

sample usage base on @guiwoda

	 /**
	 * Reverse the migrations.
	 *
	 * @return void
	 */
	public function down()
	{
		Schema::table('admin', function(Blueprint $table)
		{
			$schema_builder = Schema::getConnection()
				->getDoctrineSchemaManager()
				->listTableDetails( $table->getTable() );

			if( $schema_builder->hasIndex('admin_ibfk_1') )
				$table->dropForeign('admin_ibfk_1');

			if( $schema_builder->hasIndex('admin_ibfk_2') )
				$table->dropForeign('admin_ibfk_2');
		});
	}

@kjdion84
Copy link

kjdion84 commented Oct 10, 2017

Schema::table('table_name', function (Blueprint $table) {
    $indexes = Schema::getConnection()->getDoctrineSchemaManager()->listTableIndexes($table->getTable());
    $index_name = 'my_index_name';

    if (array_key_exists($index_name, $indexes)) {
        $table->dropIndex($index_name);
    }
});

@ryanrapini
Copy link

I just hit this issue as well. Seems like a frustrating "gotcha" that my instinct was to do

if (Schema::hasIndex('table', 'id_index')){

which doesn't work.

@kelvinphong
Copy link

I'm using mongodb, It's say 'call_user_func_array() expects parameter 1 to be a valid callback, class 'MongoDB\Database' does not have a method 'getDoctrineDriver''

@thejohn
Copy link

thejohn commented Nov 21, 2018

This has been a recurring issue since 2014, I think its time I worked on that PR.

@michaeljennings
Copy link

I hit into this issue too recently. I ended up making a using the schema macros and making a macro service provider to register a dropIndexIfExists macro and a hasIndex macro.

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

class MacroServiceProvider extends ServiceProvider
{
    public function register()
    {
        $this->registerSchemaMacros();
    }

    /**
     * Register the schema macros.
     */
    protected function registerSchemaMacros()
    {
        Blueprint::macro('dropIndexIfExists', function(string $index): Fluent {
            if ($this->hasIndex($index)) {
                return $this->dropIndex($index);
            }

            return new Fluent();
        });

        Blueprint::macro('hasIndex', function(string $index): bool {
            $conn = Schema::getConnection();
            $dbSchemaManager = $conn->getDoctrineSchemaManager();
            $doctrineTable = $dbSchemaManager->listTableDetails($this->getTable());

            return $doctrineTable->hasIndex($index);
        });
    }
}

Then in your migrations you can do the following:

Schema::table('users', function (Blueprint $table) {
    $table->dropIndexIfExists('users_index_id');

    if ($table->hasIndex('another_index')) {
        $table->dropIndex('another_index');
    }
});

@YOMorales
Copy link

@michaeljennings I changed your code to also allow for composite indexes (and generally $index as an array). Remember that Laravel allows you to do: $this->dropIndex(['my_column']);

Blueprint::macro('hasIndex', function($index): bool {
    if (is_array($index)) {
        $index = $this->createIndexName('index', $index);
    }

    $schema_manager = Schema::getConnection()->getDoctrineSchemaManager();
    $table = $schema_manager->listTableDetails($this->getTable());
    return $table->hasIndex($index);
});

Blueprint::macro('dropIndexIfExists', function($index): Fluent {
    if (is_array($index)) {
        $index = $this->createIndexName('index', $index);
    }

    if ($this->hasIndex($index)) {
        return $this->dropIndex($index);
    }

    return new Fluent();
});

@ahmadrio
Copy link

I'll have to add to this, I really don't find the way to do it with Doctrine.
I've never used DBAL, and haven't gone too far into Laravel's code to understand when it does.

@taylorotwell as you said it's easily done with DBAL, can you post a small snippet of code of how it could be done?

UPDATE
This is how it can be done:

$conn = Schema::getConnection();
$dbSchemaManager = $conn->getDoctrineSchemaManager();
$doctrineTable = $dbSchemaManager->listTableDetails('users');

// alter table "users" add constraint users_email_unique unique ("email")
if (! $doctrineTable->hasIndex('users_email_unique'))
{
    $table->unique('email');
}

thanks it works in sql server

@MarcGaumont
Copy link

MarcGaumont commented Apr 20, 2023

This should really exist... many times I find myself writing a migration and then it fails, but some of the code still executes, thus creating the indexes so when you finally end up fixing your code, if you don't go manually delete those indexes, you need to write a bunch of things like :

$connection = Schema::getConnection();
$sm = $connection->getDoctrineSchemaManager();
$indexesFound = $sm->listTableIndexes('exchange_rates');
$indexExists = false;

foreach ($indexesFound as $index) {
    if ($index->getName() == 'exchange_rates_system_exchange_rate_index') {
        $indexExists = true;
        break;
    }
}

Possibly this is a problem with laravel itself, if a migration has an error when doing the artisan migrate, it should reverse everything automatically...

@lingtalfi
Copy link

listTableDetails being deprecated, here is what we can do instead (apart waiting for this to be native to laravel):

Schema::table('events', function (Blueprint $table) {

    $schema_builder = Schema::getConnection()
        ->getDoctrineSchemaManager()
        ->introspectTable( $table->getTable() );

    if( $schema_builder->hasIndex('events_subs_admin_id_foreign') ){
        $table->dropIndex('events_subs_admin_id_foreign');
    }
});

@hafezdivandari
Copy link
Contributor

Schema::getIndexes() added on Laravel 10.37 PR #49204 and Schema::hasIndex() added on Laravel 10.43 PR #49796

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