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

Append table name to Builder to prevent conflicting IDs in SQL query #139

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

kyranb
Copy link
Contributor

@kyranb kyranb commented Sep 25, 2021

Fixes the issue "Column 'id' in where clause is ambiguous" when using additional scopes.

Example query that fails due to not specifying the table name for id:

SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'id' in where clause is ambiguous (SQL: select users.*, tenants_user.team_id as pivot_team_id, tenants_user.user_id as pivot_user_id, tenants_user.created_at as pivot_created_at, tenants_user.updated_at as pivot_updated_at from users inner join tenants_user on users.id = tenants_user.user_id where tenants_user.team_id = 2 and id = 1 and users.deleted_at is null limit 1)

Fixes the issue "Column 'id' in where clause is ambiguous" when using additional scopes.
@kyranb
Copy link
Contributor Author

kyranb commented Oct 8, 2021

@mpociot

@aitordri
Copy link

Same issue here..

@kyranb
Copy link
Contributor Author

kyranb commented Jul 11, 2022

@mpociot Can this be merged in? I've been using the fork without issue in production for almost 1 year now :)

@okaufmann
Copy link
Collaborator

@kyranb when the tests are green, we will consider merging it.

@kyranb
Copy link
Contributor Author

kyranb commented Aug 25, 2022

@okaufmann Can you take a look? I'm not that familiar with mockery, but it looks like just the table name (users) needs to be added in somewhere as the query now includes that.

@okaufmann
Copy link
Collaborator

@kyranb please merge the current master in your feature branch so the latest test actions can run again.

@kyranb
Copy link
Contributor Author

kyranb commented Aug 11, 2023

@okaufmann Sorry for the delay here Oliver. I've just done so :)

@kyranb
Copy link
Contributor Author

kyranb commented Sep 25, 2023

@okaufmann @mpociot Can this be reviewed or merged?

@kyranb
Copy link
Contributor Author

kyranb commented Nov 9, 2023

@mpociot @okaufmann Bump :)

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.

3 participants