Skip to content
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.

Sign up for GitHub

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

Jump to bottom

PostgreSQL UUID type returns a binary stream; DB to PHP conversion is always null #225

Open
llupa opened this issue Sep 28, 2023 · 5 comments · May be fixed by #229
Open

PostgreSQL UUID type returns a binary stream; DB to PHP conversion is always null #225

llupa opened this issue Sep 28, 2023 · 5 comments · May be fixed by #229
Labels
bug

Comments

@llupa
Copy link

llupa commented Sep 28, 2023

Description

As mentioned in this comment for Postgre it is recommended to use UuidType instead of UuidBinary since the platform already has its own store-mechanism for UUIDs that is not a plain VARCHAR.

UuidType when calling convertToPHPValue does not check for resource type, therefore setting the value always to null as it doesn't know what to do with it.

Steps to reproduce

  1. Create a project that uses Doctrine and this library.
  2. Setup PostgreSQL as the DB of choice.
  3. Enable uuid type for DBAL
  4. Persist and Flush an entity with a valid UUID property.
  5. Property is nullified post flush (if you allow null values).

Expected behaviour

UuidType when calling convertToPHPValue checks for resource type, in turn it is able to read bytes from Postgre and restore UUID string.

Environment details

  • version of this package: 2.0.0
  • PHP version: 8.2.10
  • PostgreSQL version: 15.3

Additional context

I already have a local fix by adding the following:

public function convertToPHPValue($value, AbstractPlatform $platform): ?UuidInterface
{
    if ($value instanceof UuidInterface) {
        return $value;
    }

    if (is_resource($value)) {
        $value = stream_get_contents($value);
    }

    if (!is_string($value) || $value === '') {
        return null;
    }

    try {
        $uuid = Uuid::fromString($value);
    } catch (Throwable $e) {
        throw ConversionException::conversionFailed($value, self::NAME);
    }

    return $uuid;
}

I noticed though that PRs are stale for quite some time and thought to make an issue first. I also don't know if this to the liking of the contributors as a solution. Am happy to open a PR if there is feedback.
@llupa llupa added the bug label Sep 28, 2023
@llupa llupa changed the title PostgreSQL UUID type returns a binary strea; DB to PHP conversion is always null PostgreSQL UUID type returns a binary stream; DB to PHP conversion is always null Sep 28, 2023
@ramsey
Copy link
Owner

ramsey commented Oct 5, 2023

Is the value returned from Postgres a resource?

@llupa
Copy link
Author

llupa commented Oct 9, 2023

I cannot tell with certainty if that is the case, I will check or make a poc for the issue. Sorry for the delayed reply, I have been traveling.

@llupa
Copy link
Author

llupa commented Oct 15, 2023

Updates 😄 , finally.

So I tried to reproduce my own report and I was failing at it. Most of the blame is on me, because while trying to debug this I changed so many things to "make it work", that I was convinced that my original report above reflected the truth.

I followed the README of this project and just adapted to PSQL if necessary. Originally the project was using uuid_binary. The result was what I have mentioned above. At some point I changed from uuid_binary to uuid and it still did not work. Unbeknownst to me at that time was that, of course it did not work, I had old data in my DB.

The is_resource did handle my now mixed data DB.

This seems to be a specific issue when you combine a DB such as posgre and uuid_binary. I made a little POC project here and here you can see the output of a small workflow, step: Proof maybe?.

This is kind of a bug? I think README can be updated to make it more clear which type to use with which which platform, instead of having a comment hidden in a closed issue from 2018.

Additionally the UuidBinary could also do a platform check and throw an incompatible exception?

public function convertToPHPValue($value, AbstractPlatform $platform): ?UuidInterface
{
    if ($platform instanceof PostgreSQLPlatform) {
        throw new IncompatibleTypeAndPlatformException;
    }
        
    // ...
}

Happy to help with either 👍

@jankal jankal linked a pull request Nov 12, 2023 that will close this issue
Open
7 tasks
@jankal
Copy link

jankal commented Nov 12, 2023

I faced the same issue triggering the following error:

The given entity of type [...] ([...]@999) has no identity/no id values set. It cannot be added to the identity map.

I'm on PHP 8.2.12 with PostgreSQL 15.3

Adding the resource handling fixed my issue:

if (is_resource($value)) {
    $value = stream_get_contents($value);
}

I am in favor of introducing resource handling for PostgreSQL and have submitted a PR #229.

@jankal jankal mentioned this issue Nov 13, 2023
Open
@jankal
Copy link

jankal commented Nov 13, 2023

I created #230 to add clarify optimal use with PostgreSQL.

@tasselchof tasselchof mentioned this issue Apr 18, 2024
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle PostgreSQL binary resources jankal/uuid-doctrine
3 participants
@ramsey @jankal @llupa