Doctrine 2 - ORM

findBy hydration with limit and offset with Oracle database (oci8 driver)

Details

  • Type: Bug Bug
  • Status: Awaiting Feedback Awaiting Feedback
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 2.3
  • Fix Version/s: None
  • Component/s: ORM
  • Security Level: All
  • Labels:
  • Environment:

Description

I tried to use the findBy method with limit and offset parameters against an Oracle database using oci8 driver.

The query seems to executed successfully but the hydrator fails when hydrating data as there is a DOCTRINE_ROWNUM column appending the "limit" clause.

Here is the exception thrown : "Notice: Undefined index: DOCTRINE_ROWNUM in [...]/vendor/doctrine/orm/lib/Doctrine/ORM/Internal/Hydration/SimpleObjectHydrator.php line 183"

I was thinking about something like this to fix this issue :

  • add an attribute (platformExtraColumns) to the platform class, storing every column added by methods like doModifyLimitQuery
  • check in hydrator method hydrateRowData if the column exists among the extra columns attribute of the custom platform
  • don't use the column if true

Maybe there is a better approach, what are your thoughts?

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Benjamin Grandfond added a comment -

I implemented it in my forks :

https://github.com/benja-M-1/doctrine2/commit/c8d899b14446accf869ddc0043f4235284375755
https://github.com/benja-M-1/dbal/commit/b9423c8d46a2bcdaa5a1f0b26a9a28259b1e44a2

It works for me, but I didn't write unit tests.

Show
Benjamin Grandfond added a comment - I implemented it in my forks : https://github.com/benja-M-1/doctrine2/commit/c8d899b14446accf869ddc0043f4235284375755 https://github.com/benja-M-1/dbal/commit/b9423c8d46a2bcdaa5a1f0b26a9a28259b1e44a2 It works for me, but I didn't write unit tests.
Hide
Permalink
Benjamin Grandfond added a comment -

Hi,

Did you have time to have a look at this issue?

Thanks

Show
Benjamin Grandfond added a comment - Hi, Did you have time to have a look at this issue? Thanks
Hide
Permalink
Christophe Coevoet added a comment -

Please send a pull request when you submit a fix. It is the proper way to submit them for review. When we want to see things waiting for review, we look at the list of pending PRs, not at all comments of the issue tracker to find links in them.

And I can tell you that this change has a big issue: it introduces a state in the database platform whereas it is currently stateless. This is likely to cause some issues when using more than 1 query (which is a common use case).

Show
Christophe Coevoet added a comment - Please send a pull request when you submit a fix. It is the proper way to submit them for review. When we want to see things waiting for review, we look at the list of pending PRs, not at all comments of the issue tracker to find links in them. And I can tell you that this change has a big issue: it introduces a state in the database platform whereas it is currently stateless. This is likely to cause some issues when using more than 1 query (which is a common use case).
Hide
Permalink
Benjamin Grandfond added a comment -

Hi Christophe thank you for your feedback.

I didn't send a PR because I wanted someone sharing his thoughts about what I suggested in this current issue. However I don't really understand the stateless argument, can you explain a bit more?

Otherwise how would do you proceed to tell Doctrine not to hydrate platform-specific columns?

Show
Benjamin Grandfond added a comment - Hi Christophe thank you for your feedback. I didn't send a PR because I wanted someone sharing his thoughts about what I suggested in this current issue. However I don't really understand the stateless argument, can you explain a bit more? Otherwise how would do you proceed to tell Doctrine not to hydrate platform-specific columns?
Hide
Permalink
Christophe Coevoet added a comment -

If you run several queries, they will be affected by the extra columns of previous requests, which is wrong

Show
Christophe Coevoet added a comment - If you run several queries, they will be affected by the extra columns of previous requests, which is wrong
Hide
Permalink
Benjamin Eberlei added a comment -

I think the ObjectHydrator catches this by skipping undefined columns, i think we might just have overoptimized the SimpleObjectHydrator a little bit.

Show
Benjamin Eberlei added a comment - I think the ObjectHydrator catches this by skipping undefined columns, i think we might just have overoptimized the SimpleObjectHydrator a little bit.

People

  • Assignee:
    Benjamin Eberlei
    Reporter:
    Benjamin Grandfond
Vote (1)
Watch (4)

Dates

  • Created:
    Updated: