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

[9.x] Fix duplicated columns on select #46049

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

vdlp-mw
Copy link
Contributor

@vdlp-mw vdlp-mw commented Feb 9, 2023

This PR is based on a previous PR (#37936) that was not merged because there were objections to the proposed fix. This PR resolves those objections.


Using $query->addSelect('something') multiple times introduces duplicated columns, causing SQLSTATE[42S21]: Column already exists: 1060 Duplicate column name 'something'.

Steps to reproduce

Execute the following query:

// accounts() is a many-to-many relation, with an intermediate Eloquent Model
App\User::find(1)->accounts()->groupBy('accounts.id')->select('accounts.*')->paginate();

The resulting problematic SQL query will be:

select count(*) as aggregate
from (select `accounts`.*,
             `accounts`.*,
             `membership`.`user_id`    as `pivot_user_id`,
             `membership`.`account_id` as `pivot_account_id`,
             `membership`.`id`         as `pivot_id`,
             `membership`.`created_at` as `pivot_created_at`,
             `membership`.`updated_at` as `pivot_updated_at`
      from `accounts`
               inner join `membership` on `accounts`.`id` = `membership`.`account_id`
      where `membership`.`user_id` = 1
      group by `accounts`.`id`) as `aggregate_table`

Proposed fix

The previous PR was denied because it used array_unique on the columns array and may result in a different column order than expected. To prevent this, a strict in_array check is used to ensure column orders are not changed. The is_array check is there for sanity purposes as columns may not always be an array. This check can be dropped once strict types are introduced.

PR includes updated test to ensure the fix works. Test failed before fix and now passes so all should be fine.

Related issues

@taylorotwell taylorotwell merged commit e92cbda into laravel:9.x Feb 9, 2023
@vdlp-mw vdlp-mw deleted the feature/fix-duplicate-select branch February 15, 2023 14:10
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

Successfully merging this pull request may close these issues.

2 participants