Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.4
    • Component/s: Platforms
    • Security Level: All
    • Labels:
      None

      Description

      Please add support for uuid datatype in PostgreSQL

        Activity

        Hide
        Darrell Hamilton added a comment -

        It seems the only missing piece is the implementation of the getGuidExpression method in the PostgreSqlPlatform class.

        Details on generating UUIDs from postgres can be found here:

        http://www.postgresql.org/docs/current/static/uuid-ossp.html

        Things that would need to be addressed:

        1) It requires the uuid-ossp module to be installed in the database. Solve with documentation or ...?

        2) There isn't just one function. Just pick one or make it configurable with a sane default?

        Show
        Darrell Hamilton added a comment - It seems the only missing piece is the implementation of the getGuidExpression method in the PostgreSqlPlatform class. Details on generating UUIDs from postgres can be found here: http://www.postgresql.org/docs/current/static/uuid-ossp.html Things that would need to be addressed: 1) It requires the uuid-ossp module to be installed in the database. Solve with documentation or ...? 2) There isn't just one function. Just pick one or make it configurable with a sane default?
        Hide
        Ross Cousens added a comment -

        I have implemented this for my own project, using what I think is a sane default:

        public function getGuidExpression()

        { return 'uuid_generate_v4()'; }

        The other issue I think that needs addressing is that the extension must be loaded into the current database upon creation. Either this is a deal-breaker right here (relying on a PGSQL plugin that's not available by default)

        OR

        If not, maybe schema:update/create needs to check for whether postgresql is being used, whether there is a guid type/generator being used in an entity, and then either execute CREATE EXTENSION uuid-ossp;, and failing that return an exception that informs the user that uuid-ossp must be available as an extension before GUID generation can be used in entities.

        Can someone explain to me please the position on this currently? Is it not ok to rely on non-core/standard functionality?

        Show
        Ross Cousens added a comment - I have implemented this for my own project, using what I think is a sane default: public function getGuidExpression() { return 'uuid_generate_v4()'; } The other issue I think that needs addressing is that the extension must be loaded into the current database upon creation. Either this is a deal-breaker right here (relying on a PGSQL plugin that's not available by default) OR If not, maybe schema:update/create needs to check for whether postgresql is being used, whether there is a guid type/generator being used in an entity, and then either execute CREATE EXTENSION uuid-ossp;, and failing that return an exception that informs the user that uuid-ossp must be available as an extension before GUID generation can be used in entities. Can someone explain to me please the position on this currently? Is it not ok to rely on non-core/standard functionality?
        Hide
        Mark Badolato added a comment - - edited

        Is there any sort of decision on this item? I was trying to use UUID with Postgres and finally got it to work by adding

        public function getGuidExpression()

        { return 'UUID_GENERATE_V4()'; }

        and went to submit a patch, then found this ticket and see that it's the exact same solution that Ross Cousens submitted above. I'd really like to not maintain my own fork of the repository just to have this change in place, and it seems like a reasonable fix (barring the uuid-ossp extension not being a Postgres default extension). Can we get this in there so it's available, and worry about the issue of informing the user about the extension at a later point?

        Or is there an easy way that anyone knows that I can add the function to my own class (in a Symfony2 project) that would extend PostgreSqlPlatform.php and add the function, without the need for me to fork Doctrine and add it on my own?

        Thanks,
        --mark

        Show
        Mark Badolato added a comment - - edited Is there any sort of decision on this item? I was trying to use UUID with Postgres and finally got it to work by adding public function getGuidExpression() { return 'UUID_GENERATE_V4()'; } and went to submit a patch, then found this ticket and see that it's the exact same solution that Ross Cousens submitted above. I'd really like to not maintain my own fork of the repository just to have this change in place, and it seems like a reasonable fix (barring the uuid-ossp extension not being a Postgres default extension). Can we get this in there so it's available, and worry about the issue of informing the user about the extension at a later point? Or is there an easy way that anyone knows that I can add the function to my own class (in a Symfony2 project) that would extend PostgreSqlPlatform.php and add the function, without the need for me to fork Doctrine and add it on my own? Thanks, --mark
        Hide
        Mark Badolato added a comment -
        Show
        Mark Badolato added a comment - Pull request submitted https://github.com/doctrine/dbal/pull/312
        Hide
        Ross Cousens added a comment -

        I hope this gets accepted but I fear it won't.

        The original complaint against implementing GUID for the PostgreSQL platform driver was because a) it required a separate extension to be enabled on the server itself b) and there were a number of GUID generation functions available.

        To use anything but v4 would be asinine unless some external constraint was forcing you to use the older generation algorithms, so I think argument b is mostly moot. Argument a can easily be solved with documentation, programmatically as well (would require more work) or just left as is because the error back from postgresql is very verbose. Cannot find function uuid_generate_v4. Google it, see that extension is required, see that it's available in PostgreSQL contrib packages, enable it, and voila it works.

        Show
        Ross Cousens added a comment - I hope this gets accepted but I fear it won't. The original complaint against implementing GUID for the PostgreSQL platform driver was because a) it required a separate extension to be enabled on the server itself b) and there were a number of GUID generation functions available. To use anything but v4 would be asinine unless some external constraint was forcing you to use the older generation algorithms, so I think argument b is mostly moot. Argument a can easily be solved with documentation, programmatically as well (would require more work) or just left as is because the error back from postgresql is very verbose. Cannot find function uuid_generate_v4. Google it, see that extension is required, see that it's available in PostgreSQL contrib packages, enable it, and voila it works.
        Hide
        Mark Badolato added a comment -

        Agreed, and I think that having SOMETHING that is usable (and has a very clear message that you need to install an extension) is better than nothing. The implementation can be expanded later to cover the extension not existing, or using something other than v4, etc. But this should be good enough for most people and it seems silly not to have, or at least not have a way to override and provide it.

        I was looking at the Symfony configs to see if there was a way to define my own platform class that could then just extend the current PostgreSqlPlatform class and add a the function, but that doesn't seem to be doable (or I just missed it)

        Show
        Mark Badolato added a comment - Agreed, and I think that having SOMETHING that is usable (and has a very clear message that you need to install an extension) is better than nothing. The implementation can be expanded later to cover the extension not existing, or using something other than v4, etc. But this should be good enough for most people and it seems silly not to have, or at least not have a way to override and provide it. I was looking at the Symfony configs to see if there was a way to define my own platform class that could then just extend the current PostgreSqlPlatform class and add a the function, but that doesn't seem to be doable (or I just missed it)
        Hide
        Mark Badolato added a comment -

        I just saw that this got merged into master. As soon as this hits 2.3* I can do away with the manual UUID generation for id's that we're currently doing

        Thanks much Benjamin!

        Show
        Mark Badolato added a comment - I just saw that this got merged into master. As soon as this hits 2.3* I can do away with the manual UUID generation for id's that we're currently doing Thanks much Benjamin!
        Hide
        Ross Cousens added a comment - - edited

        Yay, I am happy that this has been accepted and will make it through to next release. Thank you Mark/Benjamin!

        Show
        Ross Cousens added a comment - - edited Yay, I am happy that this has been accepted and will make it through to next release. Thank you Mark/Benjamin!
        Hide
        Steve Müller added a comment -

        ross neacoders This ticket is somehow in status "Awaiting Feedback". I cannot resolve this. Can you please check if you can do anything about the status. Otherwise this ticket will still show up as unresolved.

        Show
        Steve Müller added a comment - ross neacoders This ticket is somehow in status "Awaiting Feedback". I cannot resolve this. Can you please check if you can do anything about the status. Otherwise this ticket will still show up as unresolved.

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            ross neacoders
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: