#3343634: Add "json" as core data type to Schema and Database API provides the ability to use JSON columns. After that's done, we'll need the ability to add indexes for the data in those columns.
There are two ways to achieve this: indexes on SQL expressions or indexes on generated columns. Expression indexes were attempted in the issue mentioned above, but they're not supported by MariaDB, and it's not realistic to exclude such a large user base from this.
Therefore, the only remaining option is to add support for generated columns, which can then be part of indexes just like regular ones.
References:
MySql: https://dev.mysql.com/doc/refman/9.7/en/create-table-generated-columns.html
MariaDb: https://mariadb.com/docs/server/reference/sql-statements/data-definition...
PostgreSql: https://www.postgresql.org/docs/current/ddl-generated-columns.html
SQLite: https://sqlite.org/gencol.html
Allow creating generated columns in the DB Schema Definition API.
Stored vs. virtual
A stored generated column computes its value on write and keeps it on disk, like a regular column. A virtual one computes it on read and stores nothing.
Virtual columns are worth having because one of the major points of this issue is indexing. MySQL, MariaDB and SQLite can all index a virtual generated column, so you pay for the index but not for a second copy of the data. For a JSON column that's a real saving: the extracted value can be large, and it's already in the JSON document.
PostgreSQL is the exception. It only supports virtual generated columns from version 18, and it cannot index them (which also rules out unique constraints on them). Portable code should therefore use stored, and that's what the API docs recommend.
Nope.
Column::generated() adds the definition of a generated table column.Drupal\Core\Database\SchemaDefinition\GeneratedColumnStorage enumerates the possible storage options for the generated column.Nope.
TBD.
| Comment | File | Size | Author |
|---|---|---|---|
| #42 | mr-diff.patch | 7.04 KB | amateescu |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
amateescu commentedThis doesn't really need to be postponed.
Comment #3
mondrakeHow about doing this in #3411490: Replace array-based DB Schema API with a value object structure ?
Comment #4
mondrakeComment #6
mondrakeMR!15640 built on top of #3411490: Replace array-based DB Schema API with a value object structure
Comment #7
mondrakeAll 3 core supported dbs implemented. The MR is reviewable, accepting that is built on top of #3411490: Replace array-based DB Schema API with a value object structure.
Comment #8
mondrakePostponed on #3411490: Replace array-based DB Schema API with a value object structure
Comment #9
mondrakeComment #10
mondrakePostponed on #3604286: Upsert - Allow to customize the behavior of the update when the insert fails and use more Upsert queries in core to introduce an
Expressionclass.Comment #11
mondrakeComment #12
amateescu commentedThe blocker is in :)
Comment #13
mondrakeComment #14
daffie commentedComment #15
daffie commentedComment #16
amateescu commentedAddressed all the reviews, and I think this is ready now.
Comment #17
mondrakeFiled #3616311: Deprecate Column::$dbSpecificExtra and Table::$dbSpecificExtra so we can start discussing about how to make the new API really db abstract.
Comment #18
daffie commentedComment #20
amateescu commentedRemoved the pgsql guard for virtual columns from the
mainMR, and started a new one for11.xwith it included.Comment #21
mondrakeLooks very good to me now, cannot RTBC but +1 on that.
Comment #22
daffie commentedAll my remarks have been addressed on the PR.
The PR is for me RTBC.
However the IS and the CR need to be updated.
Comment #23
mondrakeComment #24
mondrakeRebased and updated CR and IS.
Comment #25
mondrakeComment #26
daffie commentedThe updated IS and CR look good to me.
For me is it RTBC.
Comment #27
catchVery excited for the possibilities this opens up.
Some of the sqlite schema introspection code is a bit painful, but unless there's some way to not expend that effort at all I doubt it can be simplified.
Apart from that, I only have one question:
What's the reason we'd ever want a virtual generated column? When reading this, I was wondering if we should add it at all, but then I realised it would probably be harder to add support later if we hard-coded everything to stored, because we'd then have to change the APIs we're adding here. So maybe that's the answer but it would be good to flesh out in the issue summary.
Comment #28
amateescu commentedVirtual generated columns are very useful when disk space is a concern. MySQL, MariaDB and SQLite can all add indexes over them, so in that case you only pay the storage price for the index, not for the data as well.
Fleshed this out in the issue summary, and addressed the feedback on the MR :)
Comment #31
catchThanks for the issue summary update + the MR changes. I can't find anything else to complain about here, so let's do it. Committed/pushed to main and 11.x, thanks!
Comment #33
driskell commentedJust wanted to share some experience here.
Generated columns are great but if you add an index to one in MySQL or MariaDB the metadata format twists slightly and becomes completely incompatible with the INSTANT alteration algorithm.
The impact is that for super large tables the resulting rebuild for any and all alterations (like adding a new column) would take hours and significant disk explosion instead of being instantaneous. Potentially meaning seemingly innocuous update code times out and takes out the site. Worse for Galera as the Total Order Isolation would subsequently take ALL connections offline until it complete.
Might be worth a word of warning somewhere as I’ve seen others bitten a few times with this.
Specifically adding an index to a generated columns is the killer. All table alterations become rebuilds and you lose all benefit from modern MySQL and MariaDB
Comment #34
amateescu commentedThat's very useful info, thanks! Do you also have some numbers for what "super large" means.. tens of millions rows, or less?
Comment #35
driskell commentedWhere I’ve seen the issue it’s been millions but it’s more I think the table size - e.g. gigabytes.
It just worth noting that with all modern engines the INSTANT kicks in most of the time so it does metadata only change and takes effect without impacting DML. But if INSTANT can’t be used and neither can INPLACE (for index on generated it can’t INPLACE either) it will hit COPY - that means copying gigabytes to a new table with no concurrent DML.
Of course there’s an element of you end up with COPY eventually anyway as there’s only so many INSTANT before it hits a limit but that’s at least one-off but if you make a table incompatible with INSTANT every single operation copies it.
Comment #36
mondrakeComment #37
xjmThis broke 11.x HEAD because it is not compatible with Posrgres 16. Per @longwave:
Comment #39
xjmPresumably we have three options:
Leaving in D12 for now.
Comment #40
catchI think we could skip the test on pgsql 16, and then any code that wants to rely on it would have to have an explicit pgsql 18 dependency.
Comment #42
amateescu commentedThat's partly what the MR for the 11.x branch was doing. Looking at the revert diff, I noticed that the MR that was targeted for
mainwas also committed to11.x, but we had a 11.x-specific MR.Reopened it, rebased on top of the latest 11.x, and attaching an interdiff between the two to clear up how we're handling pgsql 16.
Comment #43
catchKicking off postgresql runs on that MR just to double triple check before committing. My fault for committing the wrong MR to 11.x in the first place this one..
Comment #44
catchCommitted/pushed the correct MR to 11.x this time.., thanks!