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

[9.x] Prevent error in db/model commands when using unsupported columns #43635

Merged
merged 8 commits into from Aug 11, 2022

Conversation

joedixon
Copy link
Contributor

@joedixon joedixon commented Aug 10, 2022

The PR resolves #43630 #43622

The doctrine/dbal package does not support enum types when listing database columns. This PR adds support for mapping enum and sysname (issue with some distributions of sqlsrv) to string. The Artisan db:show, db:table and model:show commands have all been updated to use it.

Additonally, db:show uses the rescue helper to prevent an error if the engine type is not available. This PR also prevents the error from being reported.

@jbrooksuk jbrooksuk changed the title Fix/db commands [9.x ]Fix/db commands Aug 10, 2022
@jbrooksuk jbrooksuk changed the title [9.x ]Fix/db commands [9.x] Fix/db commands Aug 10, 2022
@mziraki
Copy link

mziraki commented Aug 10, 2022

@jbrooksuk @joedixon
model:show command needs this mapping too

@joedixon joedixon changed the title [9.x] Fix/db commands [9.x] Prevent error when using enum columns Aug 10, 2022
@joedixon joedixon changed the title [9.x] Prevent error when using enum columns [9.x] Prevent error in db/model commands when using enum columns Aug 10, 2022
@joedixon joedixon marked this pull request as ready for review August 10, 2022 09:09
@lupinitylabs
Copy link
Contributor

lupinitylabs commented Aug 10, 2022

Thanks for addressing this @joedixon ! I opened a PR at #43645 for the same issue, so I guess my PR can be closed.

There are two things to consider concerning your implementation:

  1. the issue with enum types only seems to exist on MySQL databases, so we probably might want register the type mapping only on MySQL platforms.
  2. the issue also exists for point types, so a mapping for these should probably also be added.

@joedixon
Copy link
Contributor Author

Hey @lupinitylabs - thanks for bringing the additional missing mapping to my attention. I've gone ahead and added some more that we were missing support for.

I'm not sure it's worth conditionally checking the database type as if it's only an issue with, for instance, MySQL, the other database types won't be impacted anyway.

@joedixon joedixon changed the title [9.x] Prevent error in db/model commands when using enum columns [9.x] Prevent error in db/model commands when using unsupported columns Aug 11, 2022
@lupinitylabs
Copy link
Contributor

Yes, I guess since we are just reading the structure anyways, just mapping it all to string might be fine.

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.

New command php artisan db:show returns exception on laravel version 9.24 with mysql 5.7 database
8 participants