Hi.
I advise to not use the json datatype, as some versions of MySQL and MariaDB do not support it. It can be replaced with text for a easy installation.
I'm assuming you are referring to these two fields, yes?
It might be possible to store them in text columns, but in the model, $casts them to array. I'm not sure if Laravel is able to cast arrays of objects smoothly, though.
It looks like the ->blocks references in HasBlocks and HandleBlocks may resolve fine using $casts. The three instances of ->metadatas are wrapped in json_decode, so that might work fine too.
I feel like you'd miss out on some queriability, somewhere, but I can't think of how just yet.
@IllyaMoskvin is right.
You cannot do something like this if it is not a JSON column:
$query->whereRaw("JSON_CONTAINS(thing, '[\"$thing\"]')");
or
$query->where('thing->color', 'red')
The question is if the JSON columns are ever queried like that in Twill. If not, to change them to text columns would probably expand the user base.
I, for one, can not use Twill on my production server as it uses JSON columns and the production server uses MariaDB (and it seems tricky, if not impossible, to just upgrade the SQL server).
Hi Rando, Illya, Jeremy and Emin!
While I agree as of right now we could make it work without JSON columns, we need to keep in mind that developers using Twill might want to use JSON queries on blocks content.
The JSON type is available in MariaDB since 10.2.7, which was released a year ago, in MySQL since 5.7.8, which was released almost 3 years ago now, and in PostgreSQL since 2012.
I definitely feel the pain of having to update your production server Emin, though I wouldn't recommend ignoring updates for too long as you may be subject to security issues and end of support. Which version of MariaDB are you running right now?
@ifox, I'm running CloudLinux and cPanel on the production server, and the MariaDB version is 10.1.31.
I'm sure it's still updated for security, but cPanel is not supporting a newer MariaDB version. And converting to newer MySQL, that supports JSON, seems impossible.
If the JSON columns in Twill tables provide any benefits then I'm all for it.
Maybe make it configurable? JSON is default, and with a flag in .env it creates text columns from the migration instead and adds necessary $casts to the models. Maybe?
[edit]
Actually, it seems that cPanel does, in fact, support MariaDB 10.2. I will look into upgrading.
I think making it configurable is a great idea @eminos. Not a problem for migrations, but I'd need to investigate how to conditionally add $casts to a model, which I'm sure is feasible.
I see that ->content is already being $casts to array in Block.php:
Is that enough to make it work with text as-is?
While I agree as of right now we could make it work without JSON columns, we need to keep in mind that developers using Twill might want to use JSON queries on blocks content.
I'm having a hard time thinking up a use-case for this.
I thought that one use case might be to query, "Give me all instances of a certain model (which uses HasBlocks), which have at least one 'foobar' block as part of their content."
I ended up not needing to use a JSON expression:
\App\Models\Article::whereHas('blocks', function( $query ) { return $query->where('type', 'artwork'); } )->count();
I'm not sure I understand mediables enough yet to formulate a use-case.
Hey @IllyaMoskvin,
revisiting this only now, I missed your answer here for some reason.
One use case for JSON expression would be to retrieve blocks based on their actual content. Let's say you have a block that has a radio field and you want to retrieve only the blocks where that radio field has a specific value. Definitely not the most common use case as most people would only want to loop over every blocks in the order they have been created, most likely using renderBlocks, but I can still see some interesting possibilities by keeping blocks content as a JSON column.
I'll close this for now as Twill's docs are now clear about database requirements and I don't think we want to support older databases at this point.