-
Notifications
You must be signed in to change notification settings - Fork 33
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
FIX: Don’t drop first row on repeated iteration #99
Conversation
At the risk of making your brain hate me more, we can avoid the second seek with PostgreSQL. We can specify specific rows to seek to when fetching a record (as one of the parameters to I can implement this if you want? |
Go nuts
On Fri, 28 Jun 2019 at 3:02 PM, Guy Marriott ***@***.***> wrote:
At the risk of making your brain hate me more, we can avoid the second
seek with PostgreSQL. We can specify specific rows to seek to when fetching
a record (as one of the parameters to pg_fetch_array). This means we can
make seek call pg_fetch_array($this->handle, 0, PGSQL_NUM) and then move
the processing of the result (done in nextRecord) into a separate method.
I can implement this if you want?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#99?email_source=notifications&email_token=AAAOUQE5WHRZRMW2IHMBIQTP4V5OFA5CNFSM4H4BT4F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYY5KSQ#issuecomment-506582346>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAOUQHFT2XYDHXQKGH2YDDP4V5OFANCNFSM4H4BT4FQ>
.
--
…----
Sam Minnée
CEO
SilverStripe Limited
www.silverstripe.com
Phone: +64 4 978 7334
Twitter: @sminnee <http://www.twitter.com/sminnee>
<https://www.facebook.com/silverstripe> <https://twitter.com/silverstripe>
<https://www.linkedin.com/company/431285>
|
Done :) |
code/PostgreSQLQuery.php
Outdated
pg_result_seek($this->handle, $row); | ||
return $this->nextRecord(); | ||
// Specifying the zero-th record here will reset the pointer | ||
$row = pg_fetch_array($this->handle, 0, PGSQL_NUM); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've overwritten $row
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why the 0
isn't $row
.
Also I don't understand why this doesn't increment to the next (wrong) record next time nextRecord() is called.
The nextRecord()
call after seek()
must return the same record in order to fix the bug in question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that was just a mistake 😖 . As I understand it (and from how it was working when testing it) if you call pg_fetch_array
with a row number, you set the pointer to that location and get the record at that location. pg_fetch_array
with a null row will return the row at the current pointer location and then increment the pointer.
Fixes silverstripe/silverstripe-framework#9097 Related: silverstripe/silverstripe-framework#9098 Co-authored-by: Guy Marriott <guy@scopey.co.nz>
OK @ScopeyNZ I've tested locally and it works, so 👍 from me. The only thing I did was remove the return-type hint as this patch release still supports PHP 5.6 |
Ah yep - my mistake. Only takes a few weeks of writing PHP7 code for me to change my habits apparently 😅 . I'm happy to merge considering our collab/peer review process here if you're comfortable with that (and the tests pass). |
Fixes silverstripe/silverstripe-framework#9097
Related: silverstripe/silverstripe-framework#9098