[11.x] Enable extension of connection inspection methods

[GromNaN]

In order to support 3rd party connection classes for the commands db:monitor and db:show, it's necessary to have a customization point for the methods getConnectionName and getConnectionCount. By moving the implementation to the Connection class, this implementations can be modified in 3rd party packages.

• Add Connection::getDriverTitle() with a pretty name for each driver
• Add Connection::threadsCount() and the associated grammar methods for each DBMS
• Show the connection name in the command db:show
• Deprecate DatabaseInspectionCommand methods getConnectionName and getConnectionCount.

[taylorotwell]

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

[hafezdivandari]

@taylorotwell would you please reconsider this one? @GromNaN is a member of mongodb/laravel-mongodb and this PR is needed for 3rd-party DB drivers to be able to adapt DB commands, e.g db:show - supported on their latest release 4.7. Thanks.

[driesvints]

@GromNaN can you rebase? I fixed the failing tests.

[GromNaN]

New feature: Show connection name and the driver name on distinct rows. Removing the need for pretty names.

[hafezdivandari]

adding new driver name is OK, but we still need pretty connection names.

[GromNaN]

What was displayed is a pretty version of the database type, with the detection of MariaDB by the MySQL driver. To me, the connection name is the name that is in the configuration. This is what I have changed.

I can a method Grammar::getServerType

[hafezdivandari]

I think this method should be on the Connection class like your first commit, but only returns the connection's pretty name, I suggest Connection::getTitle():

// on Connection.php
public function getTitle(): string
{
    return $this->getName();
}

// on MySqlConnection.php
public function getTitle(): string
{
    return $this->isMaria() ? 'MariaDB' : 'MySQL';
}

[GromNaN]

This name is misleading. If there are 2 MySQL connections, they would get the same title.

[hafezdivandari]

Just like before? we just want the connection name here right? just like connection version that would be the same for any mysql connection. the db:show already has database row to display the database name.

[hafezdivandari]

What about getDriverTitle()?

[GromNaN]

Ok for getDriverTitle. In that case, we don't need the new "Driver" key. Instead, name of the connection in the configuration would be useful and I would switch the Driver title and the connection name in the output.

   SQLite ............................................................. 3.46.0  
-  Driver ............................................................. sqlite  
+  Connection ................................................... myconnection  
   Database ......................................... database/database.sqlite  
   Host ......................................................................  
   Port ......................................................................  
   Username ..................................................................  
   URL .......................................................................  
   Open Connections ..........................................................  
   Tables .................................................................. 9  
   Total Size ....................................................... 76.00 KB  

[GromNaN]

@driesvints rebased and updated. If the approach seems correct, I add some tests.

[GromNaN]

Bugfix: MariaDbConnection extends MySqlConnection, so "MariaDB" is never shown currently.

[hafezdivandari]

adding new driver name is OK, but we still need pretty connection names.

[hafezdivandari]

@GromNaN It's good to have some integration tests for connection count.

[GromNaN]

I moved the function getConnectionCount to Schema\Builder. That seems to be how grammar methods are used. And this is required so that I can override this method for MongoDB (see mongodb/laravel-mongodb#3072)

[hafezdivandari]

Lgtm 👍 The only thing I would change is connectionsCount (plural) instead of connectionCount on all new methods.

[taylorotwell]

I'm not sure I fundamentally agree with re-purposing our relational database classes to support non-relational databases. Why can a Mongo package not just have it's own monitor and show commands that are Mongo specific?

[GromNaN]

We can't reproduce the entire Laravel ecosystem for MongoDB, which is why we try to implement as many of the Database and Eloquent APIs as possible. But when that API doesn't exist, it's impossible.

It's the same thing for other SQL databases like Oracle or DB2. It's easier to implement and reuse if all features are behind a well-defined API.

[hafezdivandari]

@taylorotwell IMHO, the changes here are not MongoDB specific, also valid for any other DB drivers e.g singlestore-labs/singlestoredb-laravel-driver to extend DB commands and show related info to the end-user.

It's also cleaner / more maintainable to have all queries in Grammar classes?

[alcaeus]

Obligatory disclaimer: I'm the lead engineer on the MongoDB PHP driver and a maintainer of the Doctrine MongoDB ODM.

@taylorotwell let me try to clear up what I think is some confusion here. Document databases are relational databases, it's just that relationships are different from those in "traditional" RDBMS. Without going into too much detail, there are references like in MySQL et al, but there are also embedded documents, where we don't have to extract data into a separate table/collection/store due to schema limitations. Other than that, the concepts used in relational databases are still valid in document databases.

When we decided to focus on framework integrations at MongoDB, the goal we set out to accomplish was to give people using MongoDB in Laravel (and Symfony for that matter) a good experience that comes as close to the "usual way" of working with data as they are used to it. That's one reason for leveraging Eloquent and its classes, despite the conventions used in Eloquent being very much focused on traditional databases, which has caused issues before. We could of course take the easy way out, come up with an integration for the Doctrine MongoDB ODM, and call it a day, but we believe that's not what Laravel users want. In our experience, Laravel users don't use Laravel because it's trendy or flashy, but because they like the ease of use of the framework and how it all integrates together.

Of course integrating MongoDB into eloquent isn't always easily possible. Consider the hasColumn method for example, which is straightforward in a database with a fixed schema, but when you have a flexible schema any column or field could potentially exist and the only way to be sure is to inspect every document. On the other hand, we have tooling such as this that's designed to list basic information about the database in use. I see no reason why such tooling couldn't be adapted to support any database, be it relational or not, SQL based or not, or even widely used. It's about giving developers the tools to build what they need to get their job done. Let's say someone was using Oracle or DB2, it seems unreasonable to ask them to duplicate or extend the database introspection command because separation of concern was not properly followed.

So, if I may take my MongoDB hat off and put on my engineering experience hat, the PR here does not make any changes that are unreasonable for any database system somebody may want to use with Laravel. All it does is extract conditional logic that is better kept in classes that are specific to the database in question, improving the situation for anyone using a database not supported natively by Laravel. It also improves maintainability by grouping database specific logic in classes tied to that particular database instead of keeping it in a generic class. You could compare this to platforms in Doctrine DBAL, which ensure that DBAL can support any database somebody could want to use and allow for better control on how to handle and test database specific logic.

I can't tell you to merge a pull request, and I'd be the last person to pressure maintainers into doing a particular thing. All I ask is that you look past the particular motivation that spawned this pull request (i.e. MongoDB wanting to support Laravel users), and instead consider the improvements this can bring for Laravel users and maintainers alike. Thank you.

[taylorotwell]

I'm not sure if the connection count stuff belongs in the Schema layer. It feels like a query to me and has nothing to do with building database schemas.

[hafezdivandari]

@GromNaN I suggest moving Schema\Builder::getConnectionsCount() to Connection::getThreadsCount():

// Illuminate/Database/Connection.php

public function getThreadsCount()
{
    $query = $this->getSchemaGrammar()->compileThreadsCount();

    return $query ? $this->scalar($query) : null;
}

Why schema grammar? because MySQL for instance has this data on "Performance Schema" and we already have all other information schema queries on this class.

[GromNaN]

@GromNaN I suggest moving Schema\Builder::getConnectionsCount() to Connection::getThreadsCount():

Yes, that's probably more correct since that's related to the connection.

[alcaeus]

Thanks for merging @taylorotwell!