feat: add getViews and categorize table types #3327
Conversation
|
Laravel has php artisan db:show --views
$views = Schema::getViews();To follow laravel convention Ill adjust and add getViews |
masterbater
changed the title
src/Schema/Builder.php
Outdated
| $name = $collection->getName(); | ||
|
|
||
| // Skip system collections | ||
| if (str_starts_with($name, 'system.')) { |
There was a problem hiding this comment.
This will preserve system collection from being dropped 👍🏻
Even for the gridfs bucket metadata, I think that's good to not remove this collection when dropAllTables is called.
https://www.mongodb.com/docs/manual/reference/system-collections/#database-specific-collections
There was a problem hiding this comment.
system.buckets stores data for time series collections. It's unrelated to GridFS, which stores its data in <bucketName>.fs (metadata) and <bucketName.files> (chunk data) collections.
I'm not sure how dropAllTables() is used, but I don't see why you'd need system collections to persist if everything else is being removed. Moreover, if Blueprint::drop() (called by dropAllTables()) is going to invoke Collection::drop() for each collection, you could conceivably just drop the entire database to be more efficient.
src/Schema/Builder.php
Outdated
| $name = $collection->getName(); | ||
|
|
||
| // Skip system collections | ||
| if (str_starts_with($name, 'system.')) { |
There was a problem hiding this comment.
system.buckets stores data for time series collections. It's unrelated to GridFS, which stores its data in <bucketName>.fs (metadata) and <bucketName.files> (chunk data) collections.
I'm not sure how dropAllTables() is used, but I don't see why you'd need system collections to persist if everything else is being removed. Moreover, if Blueprint::drop() (called by dropAllTables()) is going to invoke Collection::drop() for each collection, you could conceivably just drop the entire database to be more efficient.
src/Schema/Builder.php
Outdated
| foreach ($db->listCollections() as $collectionInfo) { | ||
| $collectionName = $collectionInfo->getName(); | ||
|
|
||
| // Skip system collections |
There was a problem hiding this comment.
The system collections are actual collections. In the case of system.profile, that might actually be something an application would want to query.
I'm not certain it makes sense to exclude these from all output through the integration.
| 'schema_qualified_name' => $db->getDatabaseName() . '.' . $collectionName, | ||
| 'size' => null, | ||
| 'comment' => null, | ||
| 'collation' => null, |
There was a problem hiding this comment.
If this is actually relevant, the information should be available via the options field for each collection info result. See: listCollections
| 'name' => $collectionName, | ||
| 'schema' => $db->getDatabaseName(), | ||
| 'schema_qualified_name' => $db->getDatabaseName() . '.' . $collectionName, | ||
| 'size' => null, |
There was a problem hiding this comment.
If this is relevant, the information can be provided via $collStats (MongoDB 6.2+). I'm not sure that'd be worth the overhead to obtain this for all collections/views, though.
- Group collections based on type for better organization - dropAllTables removes database
masterbater
changed the title
|
This is now a simple PR, separating tables(collections in mongodb) and views(standard views). No more segregating SYSTEM collections. For dropAllTables, I followed the recommendation to drop the entire database instead of deleting collections individually. However, this approach may lead to errors, such as "database is in the process of being dropped." While I’m unsure if tthere any production workloads that could affect this, it does cause issues in parallel testing when using the same database—an incorrect setup that should instead use separate databases for each test. |
If the library needs to work around this, it might be sufficient to catch a ServerException and no-op for DatabaseDropPending code 215. I reckon that the original approach to delete collections individually would still be subject to a race condition if a collection was created by another connection between |
| public function dropAllTables() | ||
| { | ||
| foreach ($this->getAllCollections() as $collection) { |
There was a problem hiding this comment.
@GromNaN: This is now outside the scope of this PR, but I noted that getAllCollections() could more easily use Database::listCollectionNames().
That said, I'm not even sure that method is still necessary unless you're overriding a base method. It was protected, but this seemed like the only reference to it (in this file at least).
| * In MongoDB, dropping the whole database is much faster than dropping collections | ||
| * one by one. The database will be automatically recreated when a new connection | ||
| * writes to it. | ||
| */ |
There was a problem hiding this comment.
This doc block and its method have inconsistent indentation.
Fix #3320
This fixes db:show and migrate:fresh for database that has virtual view type collections
Feat Suggestion
add getViews()
php artisan db:show --views
Checklist