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

dropAllTables #161

Merged
merged 47 commits into from
Dec 15, 2023
Merged

dropAllTables #161

merged 47 commits into from
Dec 15, 2023

Conversation

matthewjumpsoffbuildings
Copy link
Contributor

@matthewjumpsoffbuildings matthewjumpsoffbuildings commented Dec 12, 2023

Update: doing a proper step by step drop of all tables/indexes/foreign keys

As the title says - a command to drop all tables might as well just drop the database and remake it, and this makes the migrate:fresh command work properly

I am not sure if I have accessed the connection the correct way, eg DB::connection(). Also theres no tests as of yet

@taka-oyama
Copy link
Collaborator

taka-oyama commented Dec 14, 2023

Dropping all the tables by dropping the database is not ideal.
This will also drop all the backups and monitoring info such as system insight.

@matthewjumpsoffbuildings
Copy link
Contributor Author

Yea its far from ideal haha, the only other thing I can think to do is a bit of a hack (but it works) is to get the generated sql queries from all migrations up() methods, then convert all create table statements to drop table, all create index to drop index etc. You cant even use the down() methods since many internal Laravel migrations dont have any dropForeign or dropIndex statements, and annoyingly with Spanner you have to drop indexes and constraints first before dropping the table

@taka-oyama
Copy link
Collaborator

How about getting all the indexes and tables from Schema\Builder::getIndexListing(...) and Schema\Builder::getTables(...) and just drop them?

@matthewjumpsoffbuildings
Copy link
Contributor Author

That seems much better, is there a similar method for foreign keys?

@matthewjumpsoffbuildings
Copy link
Contributor Author

Also I see there is no method to compile a foreign key listing, but I can make on in the Grammar

@taka-oyama
Copy link
Collaborator

@matthewjumpsoffbuildings
Copy link
Contributor Author

Also I am noticing there is no method for getTables

https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Schema/Builder.php#L195-L200

Ye sorry my mistake, IDE wasnt picking it up for some reason

@matthewjumpsoffbuildings
Copy link
Contributor Author

matthewjumpsoffbuildings commented Dec 14, 2023

So I modified phpunit.xml to have 2 directories nodes in the testsuite, both pointing to /tests but the first uses the normal suffix Test.php, and the second uses TestLast.php. This allows any test cases that need to be run at the end to do so.

Schema/BuilderTest.php has been renamed to BuilderTestLast.php, so the dropAll test will happen right at the end

src/Schema/Grammar.php Outdated Show resolved Hide resolved
src/Schema/Builder.php Outdated Show resolved Hide resolved
src/Schema/Builder.php Outdated Show resolved Hide resolved
src/Schema/Builder.php Outdated Show resolved Hide resolved
@matthewjumpsoffbuildings
Copy link
Contributor Author

Hmm that appears not to be working without the override

@taka-oyama
Copy link
Collaborator

Seems like there's a binding missing?

1) Colopl\Spanner\Tests\Schema\BuilderTestLast::test_dropAllTables
ErrorException: Undefined array key 0

/project/vendor/laravel/framework/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php:254
/project/src/Query/Parameterizer.php:47
/project/src/Query/Parameterizer.php:46
/project/src/Connection.php:468
/project/vendor/laravel/framework/src/Illuminate/Database/Connection.php:776
/project/src/Connection.php:262
/project/src/Connection.php:239
/project/vendor/laravel/framework/src/Illuminate/Database/Connection.php:394
/project/vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php:381
/project/src/Schema/Builder.php:153
/project/tests/Schema/BuilderTestLast.php:290

@matthewjumpsoffbuildings
Copy link
Contributor Author

matthewjumpsoffbuildings commented Dec 14, 2023

Yes in reality the newer Laravel way of doing things required a $table to be passed to the compile function. I have now aligned getForeignKeys($table) and compileForeignKeys($table) with the newer approach

I have also aligned getIndexes($table) and compileIndexes($table with the standard Laravel Schema Grammar and Builder methods, instead of getIndexListing() and compileIndexListing()

@taka-oyama taka-oyama merged commit 4f175f3 into colopl:master Dec 15, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants