[Paginator]Add hidden field ordering for postgresql #640

Merged
merged 4 commits into from Jun 25, 2013

Conversation

Projects
None yet
7 participants
@denkiryokuhatsuden
Contributor

denkiryokuhatsuden commented Apr 2, 2013

In postgresql environment, when some hidden fields are used in orderBy clause,
they're not property added because $rsm->scalarMappings don't have information about them.

This change fixes above.

I'm afraid I'm not sure which branch this will be merged, but anyway here's a patch.

Add hidden field ordering for postgresql
In postgresql environment, when some hidden fields are used in orderBy clause,
they're not property added because $rsm->scalarMappings don't have information about them.
@doctrinebot

This comment has been minimized.

Show comment
Hide comment
@doctrinebot

doctrinebot Apr 2, 2013

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2385

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2385

@Ocramius

This comment has been minimized.

Show comment
Hide comment
@Ocramius

Ocramius Apr 2, 2013

Member

@denkiryokuhatsuden this kind of hotfix needs a failing test first

Member

Ocramius commented Apr 2, 2013

@denkiryokuhatsuden this kind of hotfix needs a failing test first

@denkiryokuhatsuden

This comment has been minimized.

Show comment
Hide comment
@denkiryokuhatsuden

denkiryokuhatsuden Apr 2, 2013

Contributor

Alright I will try then. Thanks for advice.

2013/04/02 19:04 "Marco Pivetta" notifications@github.com:

@denkiryokuhatsuden this kind of hotfix needs a failing test first


Reply to this email directly or view it on GitHub.

Contributor

denkiryokuhatsuden commented Apr 2, 2013

Alright I will try then. Thanks for advice.

2013/04/02 19:04 "Marco Pivetta" notifications@github.com:

@denkiryokuhatsuden this kind of hotfix needs a failing test first


Reply to this email directly or view it on GitHub.

@denkiryokuhatsuden

This comment has been minimized.

Show comment
Hide comment
@denkiryokuhatsuden

denkiryokuhatsuden Apr 3, 2013

Contributor

@Ocramius Is this acceptable?

Contributor

denkiryokuhatsuden commented Apr 3, 2013

@Ocramius Is this acceptable?

@kimhemsoe

This comment has been minimized.

Show comment
Hide comment
@kimhemsoe

kimhemsoe Apr 23, 2013

Member

If we start doing stuff like this. Why not move it the platform. Could possible also open the door of some optimization on certian platforms. Like big results and mysql makes the pagenation close to useless atm. (Mysql is very happy at creating temp tables which when big enough will end up being writened to the disk)

Member

kimhemsoe commented Apr 23, 2013

If we start doing stuff like this. Why not move it the platform. Could possible also open the door of some optimization on certian platforms. Like big results and mysql makes the pagenation close to useless atm. (Mysql is very happy at creating temp tables which when big enough will end up being writened to the disk)

@maria-p

This comment has been minimized.

Show comment
Hide comment
@maria-p

maria-p Jun 24, 2013

@Ocramius any updates on this?

maria-p commented Jun 24, 2013

@Ocramius any updates on this?

@Ocramius

This comment has been minimized.

Show comment
Hide comment
@Ocramius

Ocramius Jun 24, 2013

Member

@modeofdeath I cannot merge it myself

Member

Ocramius commented Jun 24, 2013

@modeofdeath I cannot merge it myself

guilhermeblanco added a commit that referenced this pull request Jun 25, 2013

Merge pull request #640 from denkiryokuhatsuden/patch-1
[Paginator]Add hidden field ordering for postgresql

@guilhermeblanco guilhermeblanco merged commit 20e5d98 into doctrine:master Jun 25, 2013

1 check passed

default The Travis build passed
Details
@beberlei

This comment has been minimized.

Show comment
Hide comment
@beberlei

beberlei Jun 25, 2013

Member

@kimhemsoe do you accept the challenge? I would love to see this kind of instanceof madness be moved somewhere, however its not so easy in the platforms. This code needs to be in ORM, sort of strategies for pagination.

Member

beberlei commented Jun 25, 2013

@kimhemsoe do you accept the challenge? I would love to see this kind of instanceof madness be moved somewhere, however its not so easy in the platforms. This code needs to be in ORM, sort of strategies for pagination.

@kimhemsoe

This comment has been minimized.

Show comment
Hide comment
@kimhemsoe

kimhemsoe Jun 25, 2013

Member

@beberlei Possible, can give it a shot. But you know my slowness :) So.. what you are saying is that ORM "need" some kind "platform" knowledge aswell, for lack of better term. A place where we can put platform optimizens for all of the ORM.

Member

kimhemsoe commented Jun 25, 2013

@beberlei Possible, can give it a shot. But you know my slowness :) So.. what you are saying is that ORM "need" some kind "platform" knowledge aswell, for lack of better term. A place where we can put platform optimizens for all of the ORM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment