vitorccs / laravel-csv

PHP Laravel package to export and import CSV files in a memory-optimized way
MIT License
25 stars 5 forks source link

add support for LazyCollections #5

Closed vitorccs closed 1 year ago

vitorccs commented 1 year ago

Hi @Xint0-elab

I would like to bring a point for discussion:

I have just noticed the FromQueryCursor implementation is not necessary because Laravel cursor returns a LazyCollection.

So rather than creating a new interface, we can use FromCollection - see below:

image

I have created unit tests for both use cases above, and they passed perfectly.

PS: I appreciated very much the refactor you did in FromCollection and Writer because they both enabled this library to work with LazyCollections.

Do you agree with this PR?

Xint0-elab commented 1 year ago

Looks fine, I added the FromQueryCursor just to make it simpler to migrate existing exports that use FromQuery to just implement the new interface without any other changes.

So that if you already have an export:

class MyQueryExport implements FromQuery
{
    public function query()
    {
        return Model::query()->{...complex query...};
    }
}

You only need to change FromQuery to FromQueryCursor to get the LazyCollection funcionality without any other changes:

class MyQueryExport implements FromQueryCursor
{
    public function query()
    {
        return Model::query()->{...complex query...};
    }
}
Xint0-elab commented 1 year ago

Also, with the FromQueryCursor interface counting the records executes a count query; with the FromCollection implementation, I believe the code will traverse the whole collection twice, once for counting and the second time for generating the export.

vitorccs commented 1 year ago

Also, with the FromQueryCursor interface counting the records executes a count query; with the FromCollection implementation, I believe the code will traverse the whole collection twice, once for counting and the second time for generating the export.

Good point. FromQuery runs a count query to be able to chunk (paginate):

https://github.com/vitorccs/laravel-csv/blob/73c77528fb19e3b778fe18d925b9141d1503c9e1/src/Services/Writer.php#L69

However, FromCollection outputs all contents directly without counting: https://github.com/vitorccs/laravel-csv/blob/73c77528fb19e3b778fe18d925b9141d1503c9e1/src/Services/Writer.php#L60

Also, the query is run only once, at the moment it will iterate the records: image

Xint0-elab commented 1 year ago

Hi @vitorccs, when will you tag a new release?

vitorccs commented 1 year ago

@Xint0-elab done

Xint0-elab commented 1 year ago

Thank you @vitorccs

On Wed, Apr 12, 2023 at 10:18 AM Vitor Siqueira @.***> wrote:

@Xint0-elab https://github.com/Xint0-elab done

— Reply to this email directly, view it on GitHub https://github.com/vitorccs/laravel-csv/pull/5#issuecomment-1505562421, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOO5JTPURIXLYC5K5UWVRPTXA3IT7ANCNFSM6AAAAAAW246CF4 . You are receiving this because you were mentioned.Message ID: @.***>

-- Rogelio Jacinto

M: +52 6561154714