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

[5.8] Add the ability to register custom DBAL types in the schema builder. #28214

Merged
merged 9 commits into from
Apr 17, 2019
Merged

[5.8] Add the ability to register custom DBAL types in the schema builder. #28214

merged 9 commits into from
Apr 17, 2019

Conversation

jensdenies
Copy link

Overview

This pull request aims to resolve issue #8840. In addition to that, it allows you to easily add custom, platform specific DBAL types to the framework in the future.

Quick Summary

When changing an existing column to tinyint, it will produce the following error:

DBALException in DBALException.php line 228:
Unknown column type "tinyinteger" requested. Any Doctrine type that you use has to be registered with \Doctrine\DBAL\Types\Type::addType()

Reproduction

In order to reproduce this error, you'll need to run two migrations. One should create a column and the other should should change the column:

// The first migration.
Schema::create('test_migration', function($table) {
    $table->char('test_column');
});
// The second migration.
Schema::table('test_migration', function($table) {
    $table->tinyInteger('test_column')->change();
});

Proposed Solution

In order to solve this problem, i created a "registerCustomDBALType" method in the "Builder" class. This way the platform specific builder can call this method to register their own custom type. In addition to that, the user can register their own type in their project by calling the "registerCustomDBALType" on the "Schema" facade.

@staudenmeir
Copy link
Contributor

We should also register the type on other databases, at least on SQL Server (which has a native tinyint type). Maybe even on PostgreSQL and SQLite.

Why are you overriding convertToPHPValue() and convertToDatabaseValue() without changing the implementation?

Please add a test that checks the generated SQL.

@jensdenies
Copy link
Author

jensdenies commented Apr 15, 2019

@staudenmeir I removed the unnecessary overrides and added the tests.

src/Illuminate/Database/Schema/Builder.php Outdated Show resolved Hide resolved
src/Illuminate/Database/Schema/MySqlBuilder.php Outdated Show resolved Hide resolved
src/Illuminate/Database/Schema/MySqlBuilder.php Outdated Show resolved Hide resolved
src/Illuminate/Database/Schema/Types/TinyInteger.php Outdated Show resolved Hide resolved
@jensdenies
Copy link
Author

@driesvints I made the requested changes.

@SturmB
Copy link

SturmB commented Apr 25, 2019

Hey, is there a chance someone could explain how to use this to get the dreaded "TINYINT" issue to disappear? I'm still learning and don't know how to use registerCustomDBALType properly (like where to call it, for example).

@driesvints
Copy link
Member

@SturmB this was partially reverted again in #28282 because of the following issue: #28282

@jensdenies
Copy link
Author

@SturmB @driesvints I just want to let you know that i’m still working on a solution to fix the problems with TINYINT, as well as other column types.

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.

None yet

5 participants