[5.2] Allows developers to traverse their query result via a cursor #13030

kharysharpe · 2016-04-06T00:51:45Z

Ran into an issue with attempting to use chunk wherein the underlining database would change and therefore the data would no longer be as initially requested. It is also generated a query for every chuck it returns.

Traditionally in php we would do something like below, but I found no Eloquent equivalent.

    while ($row = $stmt->fetch(PDO::FETCH_NUM)) {
      $data = $row[0] . "\t" . $row[1] . "\t" . $row[2] . "\n";
      print $data;
    }

Efficiently process thousands of records, uses memory efficiently.
Hits the database once with single query with local and global scopes.
Allows you to use a cursor to process the result set.

Usage:

foreach(Flight::where('foo', 'bar')->cursor() as $model) {

    //Do something complex with each $model

};

Update: Using generator instead of callback
Update: Using Eloquent model and changed name to cursor

Efficiently process thousands of records, uses least amount of memory, hits the database once with single query, allows you to use a ‘cursor’ to process the result set via a callback/closure.

ConnorVG · 2016-04-06T12:33:38Z

If this is to go ahead, I propose we name the function either scroll or caret as this is a cursor design (or the obvious cursor or pointer).

Fetch is too generic (and already in-use).

Set Fetch mode Switched to using generators

kharysharpe · 2016-04-06T13:03:20Z

Changed the name to traverse() as it more intuitively describes what is being done.

GrahamCampbell · 2016-04-06T13:06:09Z

Please squash to one commit.

Also, it doesn't matter if StyleCI is failing. We have it set to auto-merge the fixes whenever we merge PRs. Please see our contribution guide.

It does matter that travis is failing though.

ConnorVG · 2016-04-06T13:15:10Z

src/Illuminate/Database/Eloquent/Builder.php

+            // developer take care of everything within the callback, which allows us to
+            // keep the memory low for spinning through large result sets for working.
+
+            $continue = (yield $row);


Why do we need to assign here? Should this not just be:

if ($row === false) { return; } yield $row;

First thought was allowing for a response to be sent back, but for this use case you are correct it is not needed.

Efficiently process thousands of records, uses least amount of memory, hits the database once with single query, allows you to use a ‘cursor’ to traverse the result set.

barryvdh · 2016-04-06T13:48:01Z

@GrahamCampbell You don't have to squash your commits in a PR, you can just squash them while merging. See https://github.com/blog/2141-squash-your-commits

vlakoff · 2016-04-06T16:46:17Z

Haven't thorougly reviewed the code, but if this allows for more efficient queries on large datasets, big +1 on the idea. There should be tests for such crucial and low-level operations.

patsplat · 2016-04-07T01:52:12Z

Don't like adding new api for something that seems like it should be written the right way.

Rather than adding new api to safely iterate through a large db result set, is it possible to change the base db collection classes to fetch one row at at time?

patsplat · 2016-04-07T01:56:07Z

if there is to be new api recommend the name "cursor()" as in:

foreach( Model::where('foo', 'bar')->cursor() as $model) {

}

Would align with ->get() and ->first() as "another method that transitions a query into results"

kharysharpe · 2016-04-07T18:54:22Z

Now returns Eloquent model

phroggyy · 2016-04-10T14:32:53Z

As I can see, the only drawback is that it's impossible to eager load results. That said, I think this is a feature that should be added, and a small note made about the limitations.

tomschlick · 2016-04-14T05:06:49Z

👍 being able to query via a cursor would be very helpful for large data dumps/aggregation jobs

jbrooksuk · 2016-04-14T14:18:48Z

Cursor is the right name for this, please don't change it.

asantibanez · 2016-04-14T15:28:26Z

Could it be called "iterate"? Or something similar to " iterator"?

alibo · 2016-04-14T15:50:57Z

src/Illuminate/Database/Eloquent/Builder.php

+            //Hydrate and yield an Eloquent Model
+            $model = $this->model->newFromBuilder($row);
+
+            yield $model;


Finally, the first php yield keyword in laravel! 😀👍

tomschlick · 2016-04-14T16:42:11Z

As @jbrooksuk said, please keep it cursor.

GrahamCampbell · 2016-04-14T17:24:40Z

Could it be called "iterate"? Or something similar to " iterator"?

No.

dark-panda · 2016-04-14T18:15:41Z

One addition I'd recommend is adding a call to PDOStatement::closeCursor() somewhere in there. According to the docs at http://php.net/manual/en/pdostatement.closecursor.php...

"To ensure that the application will work with all database drivers, the author inserts a call to PDOStatement::closeCursor() on $stmt before executing the $otherStmt PDOStatement object."

JosephSilber · 2016-04-14T19:53:26Z

How would eager loading work with this?

phroggyy · 2016-04-14T19:57:50Z

@JosephSilber it wouldn't as far as I can see, as you can't retrieve any
primary key early. I think it should still be implemented, but with a note
in the docs
On Thu, 14 Apr 2016 at 21:53, Joseph Silber notifications@github.com
wrote:

How would eager loading work with this?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#13030 (comment)

JosephSilber · 2016-04-14T20:03:02Z

It kinda breaks the way Eloquent works, so I think this should only exist on the base query builder.

If you really need Eloquent objects, you can hydrate them yourself.

foreach (User::where('foo', 'bar')->toBase()->cursor() as $row) {
    $user = (new User)->newFromBuilder($row);

    // Do something complex with each $user
}

This would require a matching method on the Eloquent builder that throws an error if used directly.

BTW, generate or traverse make way more sense than cursor.

phroggyy · 2016-04-14T20:04:22Z

Agreed, it should be on the QB I guess
On Thu, 14 Apr 2016 at 22:03, Joseph Silber notifications@github.com
wrote:

It kinda breaks the way Eloquent works, so I think this should only exist
on the base query builder.

If you really need Eloquent objects, you can hydrate them yourself.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#13030 (comment)

BrandonShar · 2016-04-15T03:21:20Z

I like it as part of eloquent; it seems to me that it being part of eloquent takes care of the most common use. I feel like @JosephSilber 's example would be the majority of use cases.

kharysharpe · 2016-05-04T19:23:19Z

Thanks for the feedback and comments.

What changes, if any, are needed for this to be accepted?

taylorotwell · 2016-05-24T14:44:58Z

Just pulling this down and running DB::cursor('select * from users') throws an exception for me:

SQLSTATE[HY000]: General error: fetch mode requires the classname argument (SQL: select * from users)

Is this supposed to be working? If I can't even do that how do I know the rest of this was even tested manually?

kharysharpe · 2016-05-24T14:54:16Z

Use case was for eloquent only.

foreach(Flight::where('foo', 'bar')->cursor() as $model)  ...

I will take a look at that and fix it.

taylorotwell · 2016-05-24T15:13:59Z

It doesn't work like that either... Same error. Did you actually test that this was working on your machine?

taylorotwell · 2016-05-24T15:47:06Z

I'm fixing it all. Don't worry about it.

taylorotwell · 2016-05-24T15:50:10Z

Fixed all the problems with this. DB::cursor('sql') now returns a generator itself. The query and Eloquent versions use that. The Eloquent version returns another generator.

GrahamCampbell · 2016-05-24T16:03:53Z

:D

kharysharpe · 2016-05-24T16:15:50Z

Hmmm....that's weird thought I had renamed those already.
Thanks for fixing.
Looking forward to this being included in Laravel.

Edit:
I see what happened - parent is a2a1fb9 and not cca72f7

kharysharpe · 2016-05-24T16:27:40Z

@dark-panda mentioned using PDOStatement::closeCursor() - #13030 (comment)

Should/Could this be included as well?

taylorotwell · 2016-05-24T16:29:35Z

I’m not sure. Maybe.

On May 24, 2016, at 11:27 AM, kharysharpe notifications@github.com wrote:

@dark-panda https://github.com/dark-panda mentioned using PDOStatement::closeCursor() - #13030 (comment) #13030 (comment)
Should/Could this be included as well?

—
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub #13030 (comment)

kharysharpe · 2016-05-24T16:44:26Z

Thought about doing something like this:

            $continue = (yield $model);

            if ($continue === false) {
                 //PDOStatement::closeCursor();
                return;
            }

But this breaks HHVM tests, as it is not supported.

vlakoff · 2016-05-26T18:47:32Z

Is there documentation for this?

taylorotwell · 2016-05-26T18:48:04Z

Doesn’t look like it has been added yet.

On May 26, 2016, at 1:47 PM, vlakoff notifications@github.com wrote:

Is there documentation for this?

—
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub #13030 (comment)

vlakoff · 2016-05-26T18:49:16Z

pinging @kharysharpe, otherwise no doubt there will be little hands to take care of this.

taylorotwell · 2016-05-26T18:52:25Z

I keep a list of all things needing documentation so I can do it for 5.3 release :)

On May 26, 2016, at 1:49 PM, vlakoff notifications@github.com wrote:

pinging @kharysharpe https://github.com/kharysharpe, otherwise no doubt there will be little hands to take care of this.

—
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub #13030 (comment)

kharysharpe · 2016-05-26T19:57:08Z

@vlakoff I will make an attempt at documentation tonight.

vlakoff · 2016-06-17T18:04:03Z

Just to bring it a bit more visibility, I've added a code comment concerning fetch mode.

vlakoff · 2016-06-21T19:43:21Z

Refs #14052, which fixes support for PDO::FETCH_CLASS mode in cursor().

nerdo · 2016-07-22T19:27:05Z

+1, but generate() makes more sense to me than cursor(), as it more clearly describes what is happening under the hood at the PHP level and maintains a level of abstraction with the concept of a true DB cursor, which this isn't AFAIK.

Khary Sharpe added 3 commits April 5, 2016 19:39

Allows developers to traverse the result via a 'cursor'

7f2f695

Efficiently process thousands of records, uses least amount of memory, hits the database once with single query, allows you to use a ‘cursor’ to process the result set via a callback/closure.

Fixes based on StyleCI feedback

f63eead

Additional style fixes

ee9ec63

GrahamCampbell changed the title ~~Allows developers to traverse their query result via a cursor~~ [5.2] Allows developers to traverse their query result via a cursor Apr 6, 2016

Khary Sharpe added 3 commits April 6, 2016 07:46

Name changed as suggested on Slack

cc5faf6

Set Fetch mode Switched to using generators

StyleCI fixes

dea6794

StyleCI grrr

dcdd22c

ConnorVG reviewed Apr 6, 2016
View reviewed changes

Allows developers to traverse the result set via a cursor

d1aafe0

Efficiently process thousands of records, uses least amount of memory, hits the database once with single query, allows you to use a ‘cursor’ to traverse the result set.

Fixed botched squash

a2a1fb9

Updated to return an eloquent model and changed naming to cursor.

cca72f7

alibo reviewed Apr 14, 2016
View reviewed changes

cgwyllie mentioned this pull request May 6, 2016

Simplify chunked query result handling by using generators laravel/ideas#81

Closed

taylorotwell merged commit cca72f7 into laravel:5.2 May 24, 2016

kharysharpe mentioned this pull request Jul 28, 2016

Documentation for new cursor method. laravel/docs#2434

Merged

[5.2] Allows developers to traverse their query result via a cursor #13030

[5.2] Allows developers to traverse their query result via a cursor #13030

Conversation

kharysharpe commented Apr 6, 2016

ConnorVG commented Apr 6, 2016

kharysharpe commented Apr 6, 2016

GrahamCampbell commented Apr 6, 2016

ConnorVG Apr 6, 2016

Choose a reason for hiding this comment

kharysharpe Apr 6, 2016

Choose a reason for hiding this comment

barryvdh commented Apr 6, 2016

vlakoff commented Apr 6, 2016

patsplat commented Apr 7, 2016

patsplat commented Apr 7, 2016

kharysharpe commented Apr 7, 2016

phroggyy commented Apr 10, 2016

tomschlick commented Apr 14, 2016

jbrooksuk commented Apr 14, 2016

asantibanez commented Apr 14, 2016

alibo Apr 14, 2016

Choose a reason for hiding this comment

tomschlick commented Apr 14, 2016

GrahamCampbell commented Apr 14, 2016

dark-panda commented Apr 14, 2016

JosephSilber commented Apr 14, 2016

phroggyy commented Apr 14, 2016

JosephSilber commented Apr 14, 2016 • edited Loading

phroggyy commented Apr 14, 2016

BrandonShar commented Apr 15, 2016

kharysharpe commented May 4, 2016 • edited Loading

taylorotwell commented May 24, 2016 • edited Loading

kharysharpe commented May 24, 2016

taylorotwell commented May 24, 2016 • edited Loading

taylorotwell commented May 24, 2016

taylorotwell commented May 24, 2016

GrahamCampbell commented May 24, 2016

kharysharpe commented May 24, 2016 • edited Loading

kharysharpe commented May 24, 2016

taylorotwell commented May 24, 2016

kharysharpe commented May 24, 2016

vlakoff commented May 26, 2016

taylorotwell commented May 26, 2016

vlakoff commented May 26, 2016

taylorotwell commented May 26, 2016

kharysharpe commented May 26, 2016

vlakoff commented Jun 17, 2016 • edited Loading

vlakoff commented Jun 21, 2016

nerdo commented Jul 22, 2016

JosephSilber commented Apr 14, 2016 •

edited

Loading

kharysharpe commented May 4, 2016 •

edited

Loading

taylorotwell commented May 24, 2016 •

edited

Loading

taylorotwell commented May 24, 2016 •

edited

Loading

kharysharpe commented May 24, 2016 •

edited

Loading

vlakoff commented Jun 17, 2016 •

edited

Loading