Objection.js: joinRelation() of a m2m relation with "extra" properties fails when relation's "modify" uses an extra

[falkenhawk]

it's probably a followup of #760 and connected to changes from that time.

Person model has "movies" relation with extra: 'awesomeness' and orderBy('awesomeness', 'desc') in modify property.

await Person.query()
    .joinRelation('movies')

generates this sql:

select `Person`.* from `Person` inner join `Person_Movie` as `movies_join` on `movies_join`.`personId` = `Person`.`id` inner join (select `Movie`.* from `Movie` order by `awesomeness` desc) as `movies` on `movies_join`.`movieId` = `movies`.`id`

"awesomeness" column is inserted in wrong place, causing the following error:

Error: ER_BAD_FIELD_ERROR: Unknown column 'awesomeness' in 'order clause'

Tested with v1.0.
reproduction-template.js attached.
reproduction-template.zip

[koskimas]

Thanks for the issue and reproduction! I'll fix this for the next patch release.

[koskimas]

Actually fixing this seems to be quite challenging. The fix will probably be a breaking change and will need to wait for 2.0.

[falkenhawk]

whoops...

[koskimas]

whoops?

[falkenhawk]

I only mean i was not aware that fixing this would require that big changes, sorry. Does the issue go down to operation hooks? Or is it something else? Maybe I could help somehow?

[koskimas]

The change is not big, but moving the modify call to the right place will break some other queries. The only fix that wouldn't break things would be to copy the pivot table join to both queries. Something like this:

select `Person`.* from `Person` 
inner join `Person_Movie` as `movies_join` on `movies_join`.`personId` = `Person`.`id` 
inner join (
  select `Movie`.* from `Movie` 
  inner join `Person_Movie` as `person_join` on `person_join`.`movie_id` = `Movie`.`id` 
  order by `awesomeness` desc
) as `movies` on `movies_join`.`movieId` = `movies`.`id`

but that's somewhat stupid and inefficient.

[koskimas]

@falkenhawk Would you be ok with solving this by adding a modify/filter hook for the pivot table as suggested by @keepcosmos in #1356?

[falkenhawk]

Yes! That'd be perfect.