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] Action query method called twice, causing duplicate joins #181

Open
3 tasks done
Synchro opened this issue Oct 15, 2024 · 5 comments
Open
3 tasks done

[BUG] Action query method called twice, causing duplicate joins #181

Synchro opened this issue Oct 15, 2024 · 5 comments

Comments

@Synchro
Copy link

Synchro commented Oct 15, 2024

Prerequisites

  • Able to reproduce the behaviour outside of your code, the problem is isolated to Laravel Excel.
  • Checked that your issue isn't already filed.
  • Checked if no PR was submitted that fixes this problem.

Versions

  • PHP version: PHP 8.2
  • Laravel version: 10.41.0
  • Nova version: 4.32.11
  • Package version: LE: 3.1.52, LNE: 1.3.6

Description

I have a query method in a Nova action which extends Maatwebsite\LaravelNovaExcel\Actions\DownloadExcel:

public function query()
    {
        return $this->query
            ->join('locations', 'locations.id', '=', 'scans.location_id')
            ->leftJoin('customers', 'locations.customer_id', '=', 'customers.id')
            ->join('users', 'users.id', '=', 'scans.user_id')
            ->leftJoin('product_scan', 'scans.id', '=', 'product_scan.scan_id')
            ->leftJoin('products', 'products.id', '=', 'product_scan.product_id')
            ->leftJoin('skus', 'products.sku_id', '=', 'skus.id')
            ->leftJoin('borrowings', 'borrowings.scan_id', '=', 'scans.id')
            ->select(
                [
                    'scans.uuid',
                    'scans.status',
                    'scans.created_at',
                    'scans.external_id',
                    'users.email as user_email',
                    'customers.name as customer_name',
                    'locations.name as location_name',
                    'locations.uuid as location_uuid',
                    'skus.brand as sku_brand',
                    'skus.name as sku_name',
                    'skus.type as sku_type',
                    'products.serial as product_serial',
                    'borrowings.status as borrowing_status',
                    'borrowings.sku_type as borrowing_sku_type',
                ]
            );
    }

When I run this query standalone (e.g. in Tinkerwell), it works correctly. When I run it from within Nova, it generates an error:

Syntax error or access violation: 1066 Not unique table/alias: 'locations' (
Connection: mysql, SQL: select `scans`.`uuid`, `scans`.`status`, `scans`.`created_at`, `scans`.`external_id`, `users`.`email` as
 `user_email`, `customers`.`name` as `customer_name`, `locations`.`name` as `location_name`, `locations`.`uuid` as `location_uui
d`, `skus`.`brand` as `sku_brand`, `skus`.`name` as `sku_name`, `skus`.`type` as `sku_type`, `products`.`serial` as `product_ser
ial`, `borrowings`.`status` as `borrowing_status`, `borrowings`.`sku_type` as `borrowing_sku_type` from `scans` inner join `loca
tions` on `locations`.`id` = `scans`.`location_id` left join `customers` on `locations`.`customer_id` = `customers`.`id` inner j
oin `users` on `users`.`id` = `scans`.`user_id` left join `product_scan` on `scans`.`id` = `product_scan`.`scan_id` left join `p
roducts` on `products`.`id` = `product_scan`.`product_id` left join `skus` on `products`.`sku_id` = `skus`.`id` left join `borro
wings` on `borrowings`.`scan_id` = `scans`.`id` inner join `locations` on `locations`.`id` = `scans`.`location_id` left join `cu
stomers` on `locations`.`customer_id` = `customers`.`id` inner join `users` on `users`.`id` = `scans`.`user_id` left join `produ
ct_scan` on `scans`.`id` = `product_scan`.`scan_id` left join `products` on `products`.`id` = `product_scan`.`product_id` left j
oin `skus` on `products`.`sku_id` = `skus`.`id` left join `borrowings` on `borrowings`.`scan_id` = `scans`.`id` where `scans`.`i
d` in...

If I extract the query and just show the joins, it looks like this:

from `scans`
         inner join `locations` on `locations`.`id` = `scans`.`location_id`
         left join `customers` on `locations`.`customer_id` = `customers`.`id`
         inner join `users` on `users`.`id` = `scans`.`user_id`
         left join `product_scan` on `scans`.`id` = `product_scan`.`scan_id`
         left join `products` on `products`.`id` = `product_scan`.`product_id`
         left join `skus` on `products`.`sku_id` = `skus`.`id`
         left join `borrowings` on `borrowings`.`scan_id` = `scans`.`id`
         inner join `locations` on `locations`.`id` = `scans`.`location_id`
         left join `customers` on `locations`.`customer_id` = `customers`.`id`
         inner join `users` on `users`.`id` = `scans`.`user_id`
         left join `product_scan` on `scans`.`id` = `product_scan`.`scan_id`
         left join `products` on `products`.`id` = `product_scan`.`product_id`
         left join `skus` on `products`.`sku_id` = `skus`.`id`
         left join `borrowings` on `borrowings`.`scan_id` = `scans`.`id`

While you can see there are no duplicate joins in my query builder code, there are duplicate joins in the generated query, and that part isn't done by my code. If I replace any table name with an alias, for example:

->join('locations as locations2', 'locations2.id', '=', 'scans.location_id')

I get the same duplicate table error on the new name, even though I'm clearly only specifying it once. To confirm that these query clauses are generated from this code and not elsewhere, if I remove one join from this builder, both that join and its duplicate disappear from the resulting query.
Essentially I can't do any query that contains a join.

There is a very similar bug reported in Nova itself, but it was closed without being resolved; I don't know where the problem actually lies, whether in Nova or LNE.

I expect this code to generate a query that doesn't contain duplicate tables. I don't see how I could cause this to happen intentionally, so I can only assume it's a bug.

@Synchro
Copy link
Author

Synchro commented Oct 16, 2024

It occurred to me how this effect can be created – if the query method is called twice, the select will be overridden (and thus have no effect), but the joins will accumulate, causing the duplication. So I stuck a Log call in the method, and sure enough, it is called twice. Now to track down why...

@Synchro
Copy link
Author

Synchro commented Oct 16, 2024

Still not tracked down why this happens, but I can get the download to proceed by adding this at the top of my query method:

        static $done = false;
        if ($done) {
            return $this->query;
        }
        $done = true;

i.e. if it's been called before, don't alter the query, return it as-is. This prevents the error and the download occurs, but the CSV file contains only a header row, no data, no matter how many rows were selected.

@Synchro Synchro changed the title [BUG] Duplicate joins generated in queries [BUG] Action query method called twice, causing duplicate joins Oct 16, 2024
@Synchro
Copy link
Author

Synchro commented Oct 16, 2024

I've discovered where the duplicate call comes from. It's called once from Maatwebsite\Excel\Sheet, and then again a few lines later:

    public function fromQuery(FromQuery $sheetExport, Worksheet $worksheet)
    {
        if ($sheetExport->query() instanceof \Laravel\Scout\Builder) {
            $this->fromScout($sheetExport, $worksheet);

            return;
        }

        $sheetExport->query()->chunk($this->getChunkSize($sheetExport), function ($chunk) use ($sheetExport) {
            $this->appendRows($chunk, $sheetExport);
        });
    }

In my case $sheetExport is not an instance of \Laravel\Scout\Builder (I'm not using Scout at all), so it is called a second time before being passed to the chunk operation.

I see that this has been reported in the Laravel-Excel repo and again, but the problem wasn't fixed before being auto-closed.

The problem here is that the query() method, which is documented as the right place to alter the query before it is executed, is also used to fetch the query without altering it, as can be seen in the method above, and these uses are incompatible; if you modify the query in a non-idempotent way, it's going to break. This is a design issue.

I reinvestigated my earlier idea (with the static to track the double call) again, as I couldn't why it wouldn't work, and found that I had selected rows that resulted in no contents after the joins 🤦🏻. Selecting some others makes it workable.

@patrickbrouwers
Copy link
Member

Wouldn't putting the '$sheetExport->query()' call into a temporary variable fix it? If so, can you PR it?

@Synchro
Copy link
Author

Synchro commented Oct 16, 2024

Yes, that will probably do it. PR incoming...

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

No branches or pull requests

2 participants