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.

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

DDC-3789: Paginator does not convert entity ids if they are value objects #4632

Closed
doctrinebot opened this issue Jun 23, 2015 · 10 comments
Closed
Assignees
Labels
Milestone

Comments

@doctrinebot
Copy link

Jira issue originally created by user dadamssg:

If an entity uses a custom value object and DBAL type for its id, the value object is used as query parameters instead of the converted value.

Here is where $ids contains an array of value objects. This eventually gets set as parameter for the query here.

This leads to an exception like this:

bq. An exception occurred while executing 'SELECT a0_.id AS ID_0, a0_.created_at AS CREATED_AT_1, a0_.updated_at AS UPDATED_AT_2, a0_.name AS NAME_3, a0_.primary_image_id AS PRIMARY_IMAGE_ID_4, a0_.category_id AS CATEGORY_ID_5, a0_.created_by_id AS CREATED_BY_ID_6 FROM assets a0_ WHERE a0_.created_by_id = ? AND a0_.category_id = ? AND a0_.id IN (?, ?, ?)' with params ["9369f64a-a978-4c5c-b403-baef2576285f", "2f8d4a66-47af-4b14-9a3d-54c1debd084c", {}, {}, {}]:\n\nWarning: oci_bind_by_name(): Invalid variable used for bind

The 3 empty parameters are how my value objects are being wrongly represented.

I've fixed a similar issue in this pull request but I don't know how to go about fixing this in the Paginator.

One solution could be to only allow value objects for id's if the value object class has a **toString() method and another line gets added after the id objects are retrieved:

    /****
     * {@inheritdoc}
     */
    public function getIterator()
    {
        // existing code

        $ids = array_map('current', $subQuery->getScalarResult());
        $ids = array_map('strval', $ids);

        // existing code
    }
@doctrinebot
Copy link
Author

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1436] was assigned:
#1436

@doctrinebot
Copy link
Author

Comment created by @doctrinebot:

A related Github Pull-Request [GH-1436] was merged:
#1436

@NavyCoat
Copy link

Any ideas how to do it without using __toString Method? I'm trying to implement UuidBinary. This occurs when passing array of id's objects

@Ocramius
Copy link
Member

Doctrine uses a string representation of your object to create its internal identity map. No way around that, sorry.

@NavyCoat
Copy link

@Ocramius alright then, thanks.

@JCMais
Copy link

JCMais commented Oct 25, 2016

@NavyCoat did you found any solution?

@NavyCoat
Copy link

NavyCoat commented Oct 25, 2016

@JCMais Sorry, nothing interesting.
I currently drop idea of migration to binary uuid. I was using mysql database. But from what I found somewhere, that can works with Postgres Database where UUID is there as a type. This mean that when you pass UUID as a string, Postgres will store it as Binary UUID. (You will see it as string but there is a binary).

If you find something interesting, please share your solution here :)

@dkarlovi
Copy link

What would be an actual solution here?

Doctrine is using the __toString version to bind to a query, but here there's actually a database version and a "human readable" version, much like the types stuff. Binding anything that comes out of __toString seems wrong, it should use the type system. Am I missing something?

@knifesk
Copy link

knifesk commented Nov 2, 2018

+1

@lcobucci
Copy link
Member

Fixed by #7821

@lcobucci lcobucci self-assigned this Sep 20, 2019
Voziv added a commit to ratehub/doctrine2 that referenced this issue Oct 22, 2019
v2.6.4

[![Build Status](https://travis-ci.org/doctrine/orm.svg?branch=v2.6.4)](https://travis-ci.org/doctrine/orm)

In this release we've fixes many bugs (including a performance regression) and
made the v2.x series compatible with PHP 7.4.

--------------------------------------------

- Total issues resolved: **11**
- Total pull requests resolved: **32**
- Total contributors: **30**

Improvement
-----------

 - [7785: Fix "access array offset on value of type null" PHP 7.4 notices](doctrine#7785) thanks to @mlocati
 - [7142: Rename this repository to doctrine/orm](doctrine#7142) thanks to @greg0ire

Bug
------------------

 - [7821: Bug: doctrine#7820 paginator ignores dbal type conversions in identifiers](doctrine#7821) thanks to @Ocramius
 - [7778: Guard L2C regions against corrupted data](doctrine#7778) thanks to @umpirsky
 - [7767: PersistentCollection::matching() does not respect the collections native sorting](doctrine#7767) thanks to @stephanschuler
 - [7766: Respect collection orderBy meta when matching()](doctrine#7766) thanks to @stephanschuler
 - [7761: Do not modify UOW on PersistentCollection::clear() when owner has DEFFERED&doctrine#95;EXPLICIT change tracking policy](doctrine#7761) thanks to @paxal
 - [7750: Fix incorrect return of null values in L2C](doctrine#7750) thanks to @AlexSmerw
 - [7737: Fix MEMBER&doctrine#95;OF comparison when using criteria in query builder](doctrine#7737) thanks to @Smartel1
 - [7735: Null in fields value in Cached Entity several times on day on high-load project.](doctrine#7735) thanks to @AlexSmerw
 - [7630: Fix doctrine#7629 - `scheduledForSynchronization` leaks memory when using `@ORM\ChangeTrackingPolicy("DEFERRED&doctrine#95;EXPLICIT")`](doctrine#7630) thanks to @yethee
 - [7528: Prevent `UnitOfWork` lookup for DBAL types specified in `Doctrine\ORM\Query#setParameter()`](doctrine#7528) thanks to @Ocramius
 - [7322: JoinedSubclassPersister pass identifier types on delete](doctrine#7322) thanks to @dennisenderink and @fred-jan
 - [7266: Call to a member function resolveAssociationEntries() on boolean {"detail":"&doctrine#91;object&doctrine#93; (Symfony\\Component\\Debug\\Exception\\FatalThrowableError(code: 0): Call to a member function resolveAssociationEntries() on boolean at /www/vendor/doctrine/orm/lib/Doctrine/ORM/Cache/DefaultQueryCache.php:140)"}](doctrine#7266) thanks to @mingmingxianseng
 - [4632: DDC-3789: Paginator does not convert entity ids if they are value objects](doctrine#4632) thanks to @doctrinebot

Documentation
-------------

 - [7818: Add note into docs about not using SimpleAnnotationReader](doctrine#7818) thanks to @SenseException
 - [7791: Fix preFlush event documentation stating incorrectly that flush can be called safely](doctrine#7791) thanks to @Steveb-p
 - [7753: Add ORM annotations in getting-started docs](doctrine#7753) thanks to @SenseException and @wajdijurry
 - [7744: Fixed a typo-error](doctrine#7744) thanks to @noobshow
 - [7732: &doctrine#91;Documentation&doctrine#93; Missing comma fix](doctrine#7732) thanks to @lchrusciel
 - [7729: Update DATE&doctrine#95;ADD and DATE&doctrine#95;SUB docs](doctrine#7729) thanks to @JoppeDC
 - [7672: Added cross-links to relevant documentation](doctrine#7672) thanks to @jschaedl
 - [7612: Update ordered-associations.rst](doctrine#7612) thanks to @spirlici
 - [7610: Change APC to OPcache in improving-performance.rst ](doctrine#7610) thanks to @smtchahal
 - [7596: Correct method names and broken link in docs](doctrine#7596) thanks to @mbessolov
 - [7577: Fix of single link to dbal docs in advanced-configuration.rst](doctrine#7577) thanks to @SenseException
 - [7572: Remove codeigniter Framework example](doctrine#7572) thanks to @SenseException
 - [7571: Fix typo in inheritance mappings docs](doctrine#7571) thanks to @batwolf
 - [7557: Change Stackoverflow tag to doctrine-orm](doctrine#7557) thanks to @malarzm
 - [7551: &doctrine#91;2.6&doctrine#93; Migrate repository name doctrine/doctrine2 -> doctrine/orm](doctrine#7551) thanks to @Majkl578
 - [7530: Documentation error typo fix: s/Used-defined/User-Defined](doctrine#7530) thanks to @vladyslavstartsev
 - [7519: doctrine#7518 Fixed type mismatch between `EntityRepository#&doctrine#95;&doctrine#95;construct()` and its documented constructor arguments](doctrine#7519) thanks to @koftikes
 - [7518: `EntityRepository::&doctrine#95;&doctrine#95;construct()` expects `Doctrine\ORM\EntityManager` instead of actual required `EntityManagerInterface`](doctrine#7518) thanks to @koftikes
 - [7490: Fix broken link](doctrine#7490) thanks to @vladyslavstartsev
 - [7483: Fixed a minor syntax issue](doctrine#7483) thanks to @javiereguiluz

CI
-----------------

 - [7794: Fix test compatibility with DBAL 2.10.x-dev](doctrine#7794) thanks to @lcobucci
 - [7731: Replace custom install script with add-on](doctrine#7731) thanks to @greg0ire
 - [7473: Incremental CS checks in 2.x branches](doctrine#7473) thanks to @Majkl578
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants