-
Notifications
You must be signed in to change notification settings - Fork 398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UUID type support #1914
Comments
Oh, this is quite interesting. Some thoughts: Implementing support for DB systems with native uuid type should be easy and can probably be done quickly. I think those are Postgres, Oracle and SQL Server (as "uniqueidentifier"). Adding support for systems without native uuid type (MySQL/MariaDB, SQLite) through Looking at how query execution is done in Propel, our best option is probably to do it inside the SQL Builder and in the Formatter: flowchart LR
Z(Input) --> B
A[Model] --> |Create\nUpdate\nDelete| B[Query/Criteria]
B --> C[SQL Builder]
C --> D(DB)
D --> E[Fetcher]
E --> F[Formatter]
F --> G[Model]
F --> H[Array]
F --> I[...]
linkStyle 0 stroke-dasharray: 5 5;
linkStyle 3 stroke-dasharray: 5 5;
linkStyle 4 stroke-dasharray: 5 5;
classDef highlightNode stroke:yellow;
class C,F highlightNode;
In the SQL Builder, it should be rather easy to figure out which values of Hard to say how complicated it is to figure out which columns are uuids in the Formatter. Model columns should not be a problem, but particularly the "AS" columns seem dicey (i.e. from Instead of converting uuids in Propel, MySQL has conversion functions ( UUID_TO_BIN() and BIN_TO_UUID()) that we could use by wrapping the column and value expression into those. This would make conversion in the Formatter obsolete, however, I don't think similar function exist in SQLite, and I don't know how well we can wrap column names, so this is probably not feasible. So it looks like adding UUID support for databases without native types requires quite a bit of work, but it should give good results overall, with some edge cases that will need workarounds. And I would expect it to expose bugs in the Criteria/Criterion classes when UUIDs won't be converted. Being able to migrating a table with string-based UUIDs to a native type would take a lot of work too. But maybe I am overthinking all this, is there an easier way to do it? Probably missed tons of details too. |
Wow, nice work on getting the ball rolling I think we can also progress in small steps, partial support (e.g. only native types) could be one part Also migrating (documentation) is something we probably want to focus on as 3rd step afterwards. The main goal would be to enable this as new feature for the next release and to allow this being used as fast UUID solution for new code. |
Do we differentiate between MySQL and MariaDB? Because MariaDB also supports the native UUID type. |
That seems like a good approach.
Yes, that makes sense.
Oh, interesting, I didn't know that. Nice. Currently, there is no separate adapter for MariaDB, but this a good reason to add it. Shouldn't be too hard either (famous last words...) |
For simplicity reasons I would just treat those the same for now, using a non native approach |
What is still missing here? |
The first one is quite important, because without it, changing the flag means that swapped uuids are read as unswapped (or the other way round), effectively changing all uuids in |
Having thought about it some more, the issue with the As to the native UUID column type in MariaDB, there is a PR here #1924 which allows to use it. And I have created a PR for the documentation at propelorm/propelorm.github.com#430 |
So anything left here for the upcoming beta release now? |
I think it is all there for now |
Hi, Method "buildPkeyCriteria". This method must convert the id to binary. Old New
Without this changes no data was save by an update. |
Based on #1498
The main idea would be to add UUID as natively supported with fallback to binary(16) or alike where needed.
So all you need to do is to specify
uuid
as native type and it should use the correct/best way as per DB type.Does that make sense?
Would this be the way forward instead of defining it more manually?
The more manual way would probably have to be to support char(36), binary(16) and alike as low level definitions.
If we go with
uuid
and native support as per DB type:And feedback/ideas welcome.
The text was updated successfully, but these errors were encountered: