Add a 'uuid' database schema type

@sun It's not clear the title of issue - do you mean storage column type or database-level generation of uuid?

EDIT: I think we need to make a column type for each of supported database

Postgresql supports storage for uuid from 8.3 which is required version for D7

About generation for sqlite

It really depends on what we want to do here. Do we really want to mandate actual RFC 4122 UUIDs, or do we want to support arbitrary string identifiers, as long as they are unique.

The latter might be a little bit slower, but it would be infinitely more flexible. The initial "UUID" patch clearly wanted to support arbitrary string identifiers, that's why we went with varchar(128).

I'm personally for supporting arbitrary string identifiers everywhere. It will be definitely slower then numeric keys, but I doubt it would make any difference for any practical application. (I would welcome benchmarks proving that the difference in join performance matters for any real-life Drupal site.)

I'd suggest to provide a db-dependent implementation
Mysql - http://forums.mysql.com/read.php?24,263462,263524#msg-263524
Postgresql - http://www.postgresql.org/docs/8.3/static/datatype-uuid.html
sqlite - http://www.sqlite.org/releaselog/3_3_13.html

Actually, interoperability would be *increased* if we allow arbitrary string identifiers. Most external systems do not have a UUID key, or even a meaning full primary key to begin with.

There is a way to convert arbitrary string identifiers into a UUID (it's the whole point of UUID version 3 and 5), but that's a one way street: once you converted it there is no way back, and you have to maintain a map of ID => UUID if you need this information down the line (you usually do).

@sun: I'm not sure if that would work, as it would require sites to make this decision before they install and I'm not even sure if it's possible to replace the driver that early.

Rerolled

Thanks @estoyausente, please remove the 'Needs reroll' tag when posting a rerolled patch :)

Oh, thank for the tip. :)

Is still up for consideration for 8.0?

Regardless, we should provide the PostgreSQL equivalent as well, and probably some tests.

Added PostgreSQL equivalent.

Needs reroll after #2469941: Move database-related hook docs from system.api.php to a new database.api.php file.

re-rolling the patch but I couldn't find changes which mentioned in #17 in /core/lib/Drupal/Core/Database/Schema.php file so I left it. I feel I might be doing something wrong.
Can anyone help me with this?

@deepakaryan1988 the documentation changes contained in the patch #17 to Drupal\Core\Database\Schema were moved in #2469941: Move database-related hook docs from system.api.php to a new database.api.php file to core/modules/system/database.api.php, so this patch would need to update that file instead.

@jhedstrom Ok Thanks for the info. I will do that :)

Straight reroll of #17/#21 including documentation changes in core/modules/system/database.api.php.

A few modifications to the patch in #24:

• PostgreSQL has a native uuid type, so use it on that platform.
• On MySQL, remove the !isset($field['length']) and always enforce the length to be 36.
• Added a SQLite implementation, this just maps to the TEXT type, as SQLite has no length checking.

Patch in #25 looks good to me!

+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php
@@ -380,6 +383,8 @@ function getFieldTypeMap() {
+      'uuid:normal' => 'bytea',

Why are we not using the 'uuid' data type on PostgreSQL?

WE are not going to change this in Drupal 8. So moving this to Drupal 9.

I think this doable in minor releases, and fully BC while no data models changed

UUID functions added to https://sqlite.org/releaselog/3_31_0.html

I very much like the idea of using native support of UUID's on the database. For me the best would be if all the by Drupal supported databases would support the feature. See: #3109340: [policy] Decide whether to require json support for all database drivers for Drupal 10.
Also Microsoft SQL Server and Oracle DB support native UUID's for all currently supported versions.
The only problem is SQLite. SQLite offers support for the feature from version 3.31. Therefor we shall have to wait and what the minimum required version of SQLite will be for Drupal 10.

Setting the issue back to needs work as the patch is for D7 instead of D9.2.

Btw uuid.c included into source code base since sqlite 3.31 but I see it should be built by end user as extra extension... No idea how to use it yet

Taking a look at this this weekend. First a re-roll of the patch in #25. Note that that patch did not contain the interdiff attached there, so not including that either yet, will "re-roll" that next.

OK, here is the rerolled interdiff and the patch that actually has that applied now.

Will play around with this a bit and ideally write some tests now. We'll see.

OK, here's an updated patch. I changed the SQLITE type from TEXT to BLOB as text columns have an encoding, which is not necessary here. I am not an expert on SQLite, though, I just looked a bit through the documentation and since there wasn't a detailed reasoning for using TEXT I just went ahead and changed it.

Other than that this adds a test. It adds a basic table with a UUID column and then tests using that for a unique key and a primary key. Not sure if there is anything else we want to test. I added a new method to SchemaTest as I didn't find a specific place where are testing different schema types.

The test passes locally on all three database engines.

Reviewed whether the old patch still applies to the latest code and passes automated tests.

Steps taken to review:
1. Downloaded the latest patch file.
2. Used Git to clone local Git repository of 10.0.x of the development version that the issue pertains to.
3. The patch #47 does not apply.
Result:
So changed the issue status to Needs work.
 Added the issue tag "needs reroll".

Changing the status to Need work

Rerolled patch against #49
Attached interdiff

Move to D9 upgrade path

Fixed failed commands on #51
Attached patch against Drupal 10.0.x

Could all the changes in the patch that are not related or necessary for adding the 'uuid' database schema type be removed. They are out of scope for this issue.

+++ b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php
@@ -28,8 +28,9 @@ class Schema extends DatabaseSchema {
-   * @see \Drupal\pgsql\Driver\Database\pgsql\Schema::queryTableInformation()
    * @var array
+   *
+   * @see \Drupal\pgsql\Driver\Database\pgsql\Schema::queryTableInformation()

Patch contains a lot of useless formatting changes which should be removed

Rerolled from #47 vs cherry picking changes from following patches

Interdiff unfortunately failed to generate

Going to see if it applies before moving to NR

Yay, just 6kb

And passed

Otherwise it's rtbc, upgrade path should in conversation issues for each subsystem like fields

Thanks for breathing some new life into this @smustgrave and @andypost! 👍

Alright, updated the issue summary and created a little draft change notice in particular for database drivers, which will presumably appreciate the heads-up. Also reclassifying as a feature request and updating to 10.1.

Hopefully this can be RTBC now.

@tstoeckler thank you! Now I hope it ready for commiters

The IS needs to be updated. I have a couple of questions that I would like to see answered in the IS:
- WHY do we need a different storage type for UUID fields. Just because we can or are there other reasons to do it?
- MySQL is the most used database for Drupal. In the current patch the UUID storage will change to BINARY(36). Why this type? Are there other possibilities?
- For SQLite we are going to store it BLOB's. Why is that better than how we are storing it now?

Changing the storage for UUID fields will be a lot of work. Not only for core, but also for contrib and custom code. Site owners will be forced to to run update hooks to use the new storage for UUID field. There need to be a clear advantage and at the moment it is not clear to me that there is any. Please convince me that there is.

Is there anything we can do to prevent a BC break for the contrib database drivers for Microsoft and Oracle?

For now just a re-roll for #3315604: Move remaining database specific kernel tests to the driver modules. Will try to find some time to reply to #65 properly at some point, but just as a quick note: This patch would not change anything for core and for contrib it's just enablement to be able to experiment and validate with in contrib and eventually in core. Because the Schema API is not in any way alterable or extendable, it is necessary to take this first step in order to get something for people to build on.

Oops, this one should be better.

For SQLite we are going to store it BLOB's. Why is that better than how we are storing it now?

Re #65 There's few choices

- https://www.sqlite.org/lang_corefunc.html#randomblob for binary representation, available since 3.3.13 but for 10.x we set 3.26
- check for uuid.c extension but available since 3.31 and not every linux distro i bet

There's no difference this days about binary/text storage but generation (when running migrations) could be affected

Also there could be some analysis in which way uuids are consumed by core, as I know most of places using it as string

btw there's great answer to sqlite data-type https://stackoverflow.com/a/10105431

CR is incomplete

oracle and mssql drivers should use change record to add compatibility, for BC it could be CHAR(36)