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

Using .joins() overwrites local model fields with same named fields from joined table #681

Closed
circulon opened this issue May 22, 2022 · 4 comments
Labels
bug An existing feature is not working as intended

Comments

@circulon
Copy link
Contributor

circulon commented May 22, 2022

Describe the bug
Using .joins() will fill the fields of the Model with the values of the joined table where the joined table has fields with the same name.
So the returned hydrated Model then contains the fields that are unique to that Model as well as the values from the joined table where the field names are the same in both tables. eg id, created_at, updated_at

!!! THIS COULD SERIOUSLY AFFECT DATA INTEGRITY !!!

To Reproduce
Schemas:

company

  • id
  • name
  • phone
  • city
  • crested_at
  • updated_at

user

  • id
  • company_id
  • first_name
  • last_name
  • phone
  • city

Data:

company = {
  "id": 5,
  "name": "Acme Corp",
  "phone": "947-678-123",
  "city": "Sydney",
  "created_at": "2022-03-04 14:17:56+08",
  "updated_at": "2022-03-04 14:17:56+08",
}

user = {
  "id": 16,
  "first_name": "Bob",
  "last_ma,e": "Down",  
  "phone": "647-758-852",
  "city": "Adelaide",
}

Model query:

user = (
  User.joins("company")
    .where("company.name", "Acme Corp")
    .first()
)

print(user.serialize())

Output:

{'id': 5, 'company_id': 5, 'first_name': 'Bob', 'last_name': 'Down', 'phone': '947-678-123',  'city': 'Sydney'}

NOTE: the id, phone and city field values are from the 'company' table row not the 'user' as expected

Expected behavior
Expected output from the above scenario

{'id': 16, 'company_id': 5, 'first_name': 'Bob', 'last_name': 'Down', 'phone': '647-758-852',  'city': 'Adelaide'}

Desktop (please complete the following information):

  • OS: Mac OSX
  • Version 10.15.7

What database are you using?

  • Type: Postgres
  • Version 10.5
  • Masonite ORM 2.11.9

Additional context
Looks like there are currently no tests for join scenarios like this.

Additional observations:
The above scenario will generate a query similar to this:

SELECT * FROM "user" INNER JOIN "company" ON "user"."company_id" = "company"."id" WHERE "company"."namel" = 'Acme Corp';

The result will contain a column list like this:
id, company_id, first_name, last_name, phone, city, id, name, phone, city, crested_at, updated_at
noting here that the columns have no table prefix to denote which table they are for.

Additional Research:
apparently this is a known issue with sql queries in general and has no out of the box fix.
it is highlighted very succinctly here: https://stackoverflow.com/questions/329931/sql-select-join-is-it-possible-to-prefix-all-columns-as-prefix

Wth a possible way to handle this withing the framework from the same post here: https://stackoverflow.com/a/9926134

Possible options to handle this situation:
If we are just hydrating a model (which we are in this case) then generate a query to just get the models required fields:

  SELECT "user".* FROM "user" INNER JOIN "company" ON "user"."company_id" = "company"."id" WHERE "company"."namel" = 'Acme Corp';
@circulon circulon added the bug An existing feature is not working as intended label May 22, 2022
@josephmancuso
Copy link
Member

josephmancuso commented Jun 10, 2022

Theres no way to really do this automatically because theres no way to know which columns are the same between tables.
The only way you can do this is to add a select on the model to get both fields:

user = (
  User.joins("company")
    .select("company.id as company_pk", "company.city as company_city", "users.*")
    .where("company.name", "Acme Corp")
    .first()
)

@circulon
Copy link
Contributor Author

@josephmancuso

Thanks for this info and I do understand what you mean.

My apologies for not explaining this more thoroughly as
there is still a major problem with joins though.

In the documentation here:
https://orm.masoniteproject.com/models#joining

This notes that in its simplest form you can populate a model like this when using the @has_many decorator

User.joins('posts')

when using the @belongs_to decorator in the model you get exactly the same results as I have noted above, because the columns are not prefixed in any way.

this also produces the same results, even if you use a where on a column in the actual model.

User().join_on('company', lambda q: (
   q.where('company.name', "Acme Corp")
))

I hope this clarifies things a bit more for you and I think this issue should be reopened

Thanks for your help on this

@josephmancuso josephmancuso reopened this Jun 20, 2022
@josephmancuso
Copy link
Member

josephmancuso commented Jun 20, 2022

I'll check into the joins and join on behavior inside relationships maybe its calling something different.

For the points on the columns being the same i don't think there is a way to get around that. You will have to add the select clauses explicitly.

I don't think we can do it because:

  1. we don't know the columns on the other joining table so we can't even do an implicit select clause and join all the columns
  2. even if we did we would have to prefix every joining column. Doing this implicitly would confuse the user because the column names would all be different. I think it would be simpler to force the user to do the select(..) explicitly so they know the column names.

With that being said i will reopen so i can check the behavior of the joins and join on inside relationships

@josephmancuso
Copy link
Member

josephmancuso commented Jun 24, 2022

Going to close this. I will reopen the issue once again if you provide new information but this should be solved using the same select solution i explained earlier.

user = (
  User.joins("company")
    .select("companies.id as company_pk", "companies.city as company_city", "users.*")
    .where("companies.name", "Acme Corp")
    .first()
)

if you are doing this in a relationship you would do a select the same way

@relationship
def relation(self):
    return Model.joins("relation").select("companies.id as company_pk", "companies.city as company_city", "users.*")

Theres no way to do this automatically so the selects would have to be done manually. Could also probably be done as a scope if you want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An existing feature is not working as intended
Projects
None yet
Development

No branches or pull requests

2 participants