php artisan migrate --pretend not working on ChangeColumn

[lk77]

• Laravel Version: 8.32.1
• PHP Version: 7.2.27
• Database Driver & Version: mysql 5.7

Description:

When doing php artisan migrate --pretend i have an exception :

AddColumnNameToTableName alter table `tableName` add `columName` varchar(100) null

In SchemaException.php line 86:
                                                                  
  There is no column with name 'columnName' on table 'tableName'. 

using ->change() does not seems to work

Steps To Reproduce:

1 / add a column with a migration
2 / change that column in another migration
3 / see that second migration fail when running pretend

thanks.

[sonja-turo]

I could reproduce this, and I believe it's an issue regardless of the database or php version ( and probably other laravel versions).

I was able to address the issue by changing src\Illuminate\Database\Scheme\Grammar\ChangeColumn.php on line 27 from:

public static function compile($grammar, Blueprint $blueprint, Fluent $command, Connection $connection)
{       
    if (! $connection->isDoctrineAvailable()) {
        throw new RuntimeException(sprintf(
            'Changing columns for table "%s" requires Doctrine DBAL. Please install the doctrine/dbal package.',
            $blueprint->getTable()
        ));
    }

to

public static function compile($grammar, Blueprint $blueprint, Fluent $command, Connection $connection)
{
    if ($connection->pretending()) {
        return [];
    }
    
    if (! $connection->isDoctrineAvailable()) {
        throw new RuntimeException(sprintf(
            'Changing columns for table "%s" requires Doctrine DBAL. Please install the doctrine/dbal package.',
            $blueprint->getTable()
        ));
    }

All tests still pass, and I'd do a pull request, but I'm not confident enough in my ability to ensure this doesn't break fundamental functionality.

Edit: The issue occurs because the schema passes the change column request to the required doctrine\dbal library, and the library has no concept of pretending, so it assumes you're connecting to the DB in reality and naturally cannot find either the table or the columm.

[driesvints]

Hmm this is a tricky one. I think it's unfeasible that the pretend flag will ever work with column changes for the reasons @sonja-turo stated above.

[sonja-turo]

To add to this, yes, it's probably not feasible, because pretend does work if the table and column are already physically in the database. To confirm this:

• Make table foo with column bar manually, or via migration. Run the migration as normal to add the table to the db
• Create migration to change the column
• Run the migration with the --pretend flag

The exception will not appear because Doctrine has done physical table discovery of a previously created schema, and can find the fields it expects to see.

I think it comes down to the use case of --pretend, which really seems intended to say "I have a known set database state A, and want to ensure moving to state B works before I physically alter anything", rather than doing a pretend migration of every blueprint in one hit. If you want to do the latter, prepare an empty db and run the migrations as normal rather than pretending.

[lk77]

Well,

perhaps it's not feasible to display the query that will be done, but i think in such scenario, this exception should be catch and a simple message "query string not available" should be displayed. It should not interrupt the migration pretending process and just go on with the next one.

And for the state A / state B thing, i was in that scenario, i was migrating from version N to version N+1, which contains à lot of migrations, because a lot of thing happens during the development process between releases.

I usually use pretend to see if everything seems good to me, it's harder to spot issues on a migrated db than in pretended query logs.

[sonja-turo]

Reviewed this again, and now think @lk77 is right, and my assumptions about --pretend were wrong. This prompts me to then ask the question:
What are the historical and/or technical reasons for having ChangeColumn and RenameColumn as separate classes and not part of the *Grammar classes, where the majority of the SQL statements are generated?

In other words, what's the case for NOT building the alter statements into the grammar?

[driesvints]

Because we don't want to maintain that code ourselves. DBAL already handles that for us.

[Sandman83]

Does this mean a ticket on the DBAL repository should be opened?
I'm just wondering what is the best operational way to fix the "--pretend" option.

[lk77]

No i don't think so, there is nothing they can do,
There is no such thing as pretending in DBAL.

currently, pretend does not seems to be fixable.
As i undestand it, --pretend works only with sql generated by grammar classes,
but when DBAL is used directly, there is no way for pretend to work.

For this issue to be fixable, a major rework of the way change/rename columns are handled is needed,
which will add a maintenance burden like said by @driesvints , so i'm not sure if this possible solution will be choosen or not.

a temporary fix would be to catch that exception and display a message to the user, which will allow the pretending process to continue. the query string will be missing for that particular migration, but it's better than an unhandled exception i think.

[driesvints]

Hey al. I sent a PR where I implemented the suggestion to display a warning message. I also think that solving the underlying issues would be hard to do.

#37879