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

Artisan db:show and db:table fails when database has tables in multiple schemas #52496

Closed
bjuppa opened this issue Aug 15, 2024 · 3 comments · Fixed by #52501
Closed

Artisan db:show and db:table fails when database has tables in multiple schemas #52496

bjuppa opened this issue Aug 15, 2024 · 3 comments · Fixed by #52501

Comments

@bjuppa
Copy link

bjuppa commented Aug 15, 2024

Laravel Version

11.11.0

PHP Version

8.3.10

Database Driver & Version

SQL Server 15.00.2000

Description

For databases with tables organised in more than one schema/namespace (e.g. SQL Server, Postgres) the command php artisan db:show --counts fails with a QueryException.

Just php artisan db:show without counts works fine, but php artisan db:table won't list correct column information from a table in a non-default schema, but at least it doesn't throw errors.

The current code is not referencing tables with a fully qualified name, which is required for tables in a non-default schema.

Should we bother to fix this?

Honestly, I'm hesitant because this use-case is so narrow this may not even be worth fixing. Almost nobody (except me apparently) works with a legacy database that has some tables in non-default schemas.

The case for it though is that it's exactly when poking around in legacy databases that artisan's db:show and db:table commands really shine. They provide a quick way to inspect the existing db without firing up a database management tool.

Where it breaks

For the relevant databases, schema is already available in the $table array, but is not added to the name when counting rows in ShowCommand::tables() function here:
https://github.com/laravel/framework/blob/11.x/src/Illuminate/Database/Console/ShowCommand.php#L82

In the TableCommand::handle() function, the list of tables shown to the user does not include schema names, and $tableName is extracted from user input and used directly to extract table data without fully qualifying it.

Suggested solutions

Now, I have some suggested solutions and would like some feedback...

Naive solution:

If schema is available in the $table array, just join it in front of name, separated with a dot right in the commands.

If a db would have a different pattern than schema.table for qualifying table names, this would break. But I guess that is SQL standard so probably not an issue.

But this solution will also break when the table name has spaces or special characters in it. I don't see any escaping or quoting of table names happening deeper down in the query builder. Come to think of it, the current implementation would most likely break too if a table name has special chars, because there's no quoting of the table name in the call to count.

More robust solutions:

  1. Let every grammar's implementation of compileTables() not only extract name & schema but also qualifiedName, properly quoted for the specific database. Then use that for the calls to get additional table information. Here's an example what this query looks like in the PostgresGrammar.
  2. Let every grammar implement a function similar to PostgresGrammar::escapeNames() that can be used to combine the schema & name for the calls.

I'm leaning towards solution 1, What do you think?

Steps To Reproduce

  1. Fire up a new Laravel app with a db that is supporting schemas, like Postgres or MS Sql.
  2. Create an empty database and set connection details in '.env'.
  3. Manually create a table with some columns in another namespace. In MS Sql that can be done with:
CREATE SCHEMA another;
CREATE TABLE another.dimension (
    id INT IDENTITY(1,1) PRIMARY KEY
);

Then, after setup:

  • Run php artisan db:show to see that table listed.
  • Run php artisan db:show --counts to get an error.
  • Run php artisan db:table to see missing information for that table.
@bjuppa
Copy link
Author

bjuppa commented Aug 15, 2024

Bonus, if you want to be really fancy you can create a table name with a space in MS Sql using:

CREATE TABLE [dimension B] (
    id INT IDENTITY(1,1) PRIMARY KEY
);

Interestingly, this doesn't seem to break the db:show or db:table commands as long as the table is in the default schema.

@hafezdivandari
Copy link
Contributor

@bjuppa I've sent a PR #52501 to fix this. Meanwhile I'm testing myself, would you please check if it resolves your issue?

PS: having space in the table name + schema name and / or prefix shouldn't have been a problem since I've already fixed that on #50019 (related to #49965 and #50855)

@bjuppa
Copy link
Author

bjuppa commented Aug 15, 2024

Thank you @hafezdivandari, that was a quick response! I'll test your PR when I get into the office tomorrow.

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 a pull request may close this issue.

2 participants