Database Schema Grammars Macroable

[SebastianS90]

I would like to add custom column types in my migrations.

The Blueprint class is already Macroable, so it is easy to add a new type there. Now I need is to add the typeXyz() functions to all grammars I would like to support. Extending the grammar is not possible without also extending the corresponding connection. So it would be nice if the grammars in Illuminate\Database\Schema\Grammars could be macroable, too. Probably each grammar individually needs to pull in the Macroable trait, such that I can give a different implementation for each database system.

This is a better solution than raw statements in migrations, because I can support different database systems.

[ragingdave]

Would you be able to give of an example of what you are looking to accomplish? I would say that perhaps you are trying to add functionality that shouldn't exist in the Grammars. I would take Grammars to be something that is specific to the connection/database you are using. I guess an example would be adding a complex type that has different underlying column types? but even then.... you could end up making your type incompatible with all database connections.

[SebastianS90]

Example:

// This already works, but results in an exception because the grammar has no `typeBinary16` method:
Illuminate\Database\Schema\Blueprint::macro('binary16', function ($column) {
    $this->addColumn('binary16', $column);
});

// This would be nice:
Illuminate\Database\Schema\Grammars\MySqlGrammar::macro('typeBinary16', function() {
    return 'binary(16)';
});

In my case, I like the mysql binary(16) type because it is the optimal type to store an inet_pton encoded ip address. Laravel's binary migration type creates a blob instead, which is not as performant as a fixed size binary in mysql.

Making MySqlGrammar (and also the other schema grammars) macroable provides an easy way to add custom types.

[ragingdave]

I would almost argue that it might be more prudent to actually make the type binary.....be binary and not a blob. I'm not sure what the original reason was for using blob instead of binary and add an additional blob type.

Since this is potentially the only use case, because it's the only type I can see that is straight up changed from the default mysql types, I would probably argue against being able to macro db types. It just really doesn't feel like a place that should be macroable, for the reason of you can already do 99% of what's potentially needed (custom Blueprint macros). Additionally, It introduces potential fragmentation into the package space where if a package has a custom type, then it needs to add that type to any databases it would support rather than supporting all db's out of the box. Honestly I would just do something similar to https://github.com/spatie/laravel-binary-uuid and potentially try and start the conversation around why binary is blob and not binary in the first place.

Also, Also adding custom types is like saying you are adding a new type to the database engine itself which clearly a php framework won't be capable of doing so having the ability to seems wrong.

[SebastianS90]

I strongly recommend against changing existing types, because it breaks a lot. Databases that initially were migrated using an old version of laravel have different column types than freshly migrated databases. This should never happen and is the reason why you should not edit existing migrations (rather add new migrations that change your columns as required).

Threre are probably more types or other DB specific features, e.g. in postgres. The Laravel core should only support types that can be used in all supported database systems. A custom application however should be able to add custom types and thereby drop support for other database systems.

The uuid package uses the typical child class approach. This is ok, but has two downsides:

• it is more code than just some ::macro
• you cannot combine two such packages that both want to replace the grammar with their own subclass

[ragingdave]

I strongly recommend against changing existing types, because it breaks a lot.

Totally 100% agree except in maybe this instance. If a binary type isn't a binary type, then the world doesn't really make sense, again not 100% sure what the original reason was but still. For this particular change, I would argue that it would fix more than it broke.....yes every migration in the world would have to have a find/replace ran to replace binary with blob.....but then it's over. There are probably other reasons not to do it but we are getting off track. or maybe a param that toggles blob vs binary and defaults to blob for now.

I think the main issue is that binary as a type isn't available, so rather than being able to macro custom types, adding a new type like blob that creates a binary to make it so that all types of a db engine are represented will be more maintainable in the long term. It would create less confusion and package fragmentation as I said before.

Imagine a dev puts out a package, that is exactly what hypothetical me is looking for functionality wise but because you were using it in your project with postgres.....suddenly I can't use it unless you went out of your way to ensure it was compatible with all available databases. Maybe you don't have time or have moved on from that project, then one of 2 things happen, someone forks it and maintains a proper version of it, most likely breaking your version in the process to remove the postgres hyper-specific logic...or it just withers and dies as a package.

You are talking about adding types that exist in the engines.....so why not add those types to the Grammars directly and not mess about with being able to have a new type like lollipops that's actually some random type that no one uses or worse yet, duplicates a type because it sets a default like 16 for the length.

Bottom line, and maybe I'm the only one, but macro-ing db specific grammar types just feels like the wrong solution to a very small problem (at least as far as our example cases here have taken us). I'm not denying that there are probably types that aren't database-agnostic, but I think the first step is to add them to the Grammars so everyone can benefit from them being fleshed out.

[SebastianS90]

Implemented in laravel/framework#24513 (Schema Grammars extend the Database Grammar) 🎉