Skip to content
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

[Bug]: WithChunkReading concern overrides WithReadFilter concern #4267

Open
1 task done
craigpipeandpiper opened this issue Feb 7, 2025 · 2 comments
Open
1 task done
Labels

Comments

@craigpipeandpiper
Copy link

craigpipeandpiper commented Feb 7, 2025

Is the bug applicable and reproducable to the latest version of the package and hasn't it been reported before?

  • Yes, it's still reproducable

What version of Laravel Excel are you using?

3.1.56

What version of Laravel are you using?

10.48.23

What version of PHP are you using?

8.3.15

Describe your issue

When an import class uses both the WithChunkReading and the WithReadFilter concern, the WithReadFilter concern does not work because chunk reading is also implemented by a PhpSpreadsheet read filter. As it seems there can only be one read filter at a time, this breaks the import.

WithReadFilter combined with WithChunkReading concerns is the only workaround for the issues identified in previous bug #4252 where PhpSpreadsheet provides read filters as a way to mitigate memory consumption of large sheets.

How can the issue be reproduced?

Add a simple read filter to an import process with the WithReadFilter concern, along with WithChunkReading concern (add the WithChunkReading concern before WithReadFilter).

To verify, add a dd() to the readCell() method of the IReadFilter interfaced class.

What should be the expected behaviour?

Both concerns should work, or an exception should be thrown to make clear the concerns are mutually exclusive.

@craigpipeandpiper
Copy link
Author

It seems that PhpSpreadsheet supports only one read filter at a time:

    public function setReadFilter(IReadFilter $readFilter)
    {
        $this->readFilter = $readFilter;

        return $this;
    }

The fix would be either to call the existing read filter from inside ChunkReadFilter if both concerns exist, or to throw an exception if this is not something you want to support.

@patrickbrouwers
Copy link
Member

patrickbrouwers commented Feb 19, 2025

I guess a PR to use the reader filter provided by the concern instead of the ChunkReadFilter in ReadChunk#182 would be okay. I'm not sure how well it would work in combination with the rest of the chunk reading logic, but I guess if someone wants to overrule that somehow, that'd be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants