Add type to store UUIDs as binary (UUID_BINARY)

[mringler]

This allows Propel to work with UUIDs stored in a binary data column like BINARY(16) in MySQL.

To use it, the column type in schema.xml has to be set to UUID_BINARY:

    <table name="my_table">
        <column name="uuid" required="false" type="UUID_BINARY"/>
    </table>

In models, fields of that type will always contain the UUID as string, conversion between binary value and string is done during loading (in hydrate()) or saving (in doInsert() or buildCriteria() from doUpdate()). Similar conversion happens in the findBy methods in the query class.
This is a different approach than planned in #1914, but I found it integrates better into Propel.

Per default, UUID conversion will swap some bytes around, in accordance with MySQL's uuid_to_bin() function. This allows better indexing for version-1 UUIDs. The default behavior can be changed in schema.xml by setting the value of UuidSwapFlag in vendor information:

<?xml version="1.0" encoding="ISO-8859-1" standalone="no"?>
<database name="bookstore" defaultIdMethod="native"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    namespace="Propel\Tests\Bookstore"
>
    <vendor type="mysql">
          <parameter name="UuidSwapFlag" value="false"/>
    </vendor>

Caveats:

• Feature is in experimental state. It is tested, but only to an extend.
• No migration of columns with data at the moment (generated "ALTER TABLE" statements will fail).
• Changing UuidSwapFlag currently does not trigger a migration at all (it should cause all existing UUIDs to be (un)swapped).
• Going around the model classes, for example by using select() on the query class or manually setting a formatter like SimpleArrayFormatter, will leave UUIDs in their binary form and users have to convert them manually by calling UuidConverter::binToUuid($uuidData, $swapFlag) (depending on PDO behavior for the underlying dbms, they will have to read the resource/stream into a string first by calling stream_get_contents($uuidData)).
• Similarly, if for some reason the query class fails to detect UUID input and does not automatically convert it into the binary form, users will have to do it manually by calling UuidConverter::uuidToBin($uuid, $swapFlag).

Realistically, this will take a brave user to test and give feedback.

[dereuromark]

Great work!

type="UUID_BINARY"

vs #1915 and type="UUID"

My main question would be: Do we need to have different type names per DB?
From a user perspective the internals (and custom decisions on the specific sub type) are not so interesting usually, so lets say you want to use UUID from a framework perspective and support 3+ DB types
The ORM ideally abstracts the internals away, that you can specify UUID across your project definitions then and they dont have to be different per DB type.

In other words: Ideally we define

type="UUID"

and by default picks the best internal strategy by default for each ORM DB type supported.

Is that something we can achieve?

[codecov-commenter]

Codecov Report

Base: 88.44% // Head: 73.25% // Decreases project coverage by -15.19% ⚠️

Coverage data is based on head (ce0a677) compared to base (b35fb89).
Patch coverage: 65.73% of modified lines in pull request are covered.

Additional details and impacted files

@@              Coverage Diff              @@
##             master    #1917       +/-   ##
=============================================
- Coverage     88.44%   73.25%   -15.20%     
- Complexity     7854     7917       +63     
=============================================
  Files           224      227        +3     
  Lines         20988    21133      +145     
=============================================
- Hits          18562    15480     -3082     
- Misses         2426     5653     +3227     

Flag	Coverage Δ
5-max	73.25% <65.73%> (-15.20%)	⬇️
7.4	73.25% <65.73%> (-15.20%)	⬇️
agnostic	?
mysql	68.95% <62.94%> (-0.14%)	⬇️
pgsql	69.05% <56.57%> (-0.01%)	⬇️
sqlite	66.96% <51.39%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Propel/Generator/Model/ForeignKey.php	89.73% <ø> (-4.97%)	⬇️
src/Propel/Generator/Model/PropelTypes.php	85.71% <ø> (-14.29%)	⬇️
src/Propel/Generator/Model/Schema.php	50.00% <0.00%> (-47.71%)	⬇️
...erator/Platform/Util/MysqlUuidMigrationBuilder.php	0.00% <0.00%> (ø)
src/Propel/Generator/Reverse/MysqlSchemaParser.php	88.26% <ø> (ø)
src/Propel/Runtime/Map/ColumnMap.php	55.44% <0.00%> (-25.75%)	⬇️
src/Propel/Generator/Model/Column.php	80.65% <57.14%> (-16.85%)	⬇️
src/Propel/Runtime/Util/UuidConverter.php	66.66% <66.66%> (ø)
...rc/Propel/Generator/Model/Diff/TableComparator.php	96.74% <75.00%> (-3.26%)	⬇️
src/Propel/Generator/Model/Table.php	73.47% <79.16%> (-17.94%)	⬇️
... and 135 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[mringler]

Ok, now UUID_BINARY is used as fallback type for UUID in MySQL and SQLite (all other vendors have a native UUID type). So now the UUID type can always be used (even though the result might be surprising).

I think we should keep both types, as it allows to still use UUID_BINARY on systems where UUID maps to the native type. This might be useful if an old version of the dbms is used or if the db already uses binary columns and cannot be changed. Also, if you know your database will use UUID_BINARY anyway, you might find it cleaner to just specify that. And should MySQL ever support a native UUID type and we change the behavior of UUID, users can switch to UUID_BINARY if they want to avoid migrating their db.

@dereuromark what do you think does that make sense?

[dereuromark]

Unsafe usage of new static().

These should be in phpstan ignoreErrors list.

[mringler]

I have added code for automatic migration between CHAR/VARCHAR and BINARY uuid columns on MySQL. If column type is changed in schema.xml, Propel will add code to the migration file, which creates a new column for the converted values and then replaces the old column with that new one.

When changing from a binary column to a CHAR type, propel does not know that the column contains uuids, it just sees binary to char. To let Propel know that it is a conversion of uuids, the column has to be marked as such by adding the content="UUID" to the declaration in schema.xml (we can use something else if that is better):

<column name="id" type="varchar" size="36" content="UUID" />

I had to change some overall behavior to get this working, my assumption is that these changes are useful:

• In MySQL schema parser, binary types were mapped to BLOBs, now they map to the binary type.
• When detecting schema changes, a foreign key is considered to have changed when the type of one of the involved columns change (leads to dropping and recreating the Fk constraint).
• Propel combines ALTER TABLE statements in migrations, I have moved that code into its own class (AlterTableStatementMerger) and you can add a statement, which tells the merger not to combine statements above with those below (AlterTableStatementMerger::NO_MERGE_ACROSS_THIS_LINE). Also the merger recognizes DML statements and will not merge across them.
• I refactored some statements I ran into because I found them hard to read. I try to not do that, as it tends to obfuscates commits, but didn't want to remove them either.
• The tests only run on MySQL, not MariaDB, so they would not have run on Github. But I do want those tested not just locally, so I changed ci to use MySQL instead of MariaDB.

With the test, I am becoming more confident that this might actually work. Let me know what you think!

[asmarovydlo]

Great work!

[dereuromark]

Lets put cleanup into follow up PR
Nice work