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

Support write LEFT JOIN which include more than one condition use AND? #1048

Open
zw963 opened this issue Jun 21, 2024 · 7 comments · May be fixed by #1050
Open

Support write LEFT JOIN which include more than one condition use AND? #1048

zw963 opened this issue Jun 21, 2024 · 7 comments · May be fixed by #1050

Comments

@zw963
Copy link
Contributor

zw963 commented Jun 21, 2024

For discusstion, check #1901 and optional this

Following is a SQL example which i expected to generate.

university has_many chong_wen_baos
chong_wen_bao belongs_to university

SELECT universities.* FROM universities 
LEFT JOIN chong_wen_baos 
ON 
universities.id = chong_wen_baos.university_id 
AND 
chong_wen_baos.user_id = 1;

As you can see, i use a AND condition with the LEFT JOIN ON, Above SQL output expected result, which includes all university, whatever whether there is record exists on the right table (the select chong_wen_baos.id is null if no right table record exists)

I don't know how to write SQL like above, current, I am using like this, but not work.

query = query
      .where_chong_wen_baos(ChongWenBao.new.user_id(current_user.id), auto_inner_join: false)
      .left_join_chong_wen_baos

It generate SQL like this: (AND replaced with WHERE)

SELECT universities.* FROM universities 
LEFT JOIN chong_wen_baos 
ON universities.id = chong_wen_baos.university_id 
WHERE 
chong_wen_baos.user_id = 1;

this is not what i want, because it is not return the left table records which no right table record exists.

I thought one of solution is, write join like the where method?

query = UniversityQuery.new
query.join("LEFT JOIN chong_wen_baos on universities.id = chong_wen_baos.university_id AND chong_wen_bao.user_id = ?", current_user.id)

Thanks.

@zw963
Copy link
Contributor Author

zw963 commented Jun 21, 2024

Hi, Because I am eager to fix a bug that has been present in my project for a long time, so, i have to try solve this issue use a really dirty hack. please have a look.

#1049

it's seem like work, the only downside is, i have to write join("....") manually before any where_chong_wen_baos, otherwize, no join SQL statement generated.

following is PART OF complicated usage on my project (Please check the second to last line)

    cwb_query = ChongWenBaoQuery.new
    query = UniversityQuery.new.preload_chong_wen_baos(cwb_query).preload_city

    if q.presence
      if q.matches? /^\d{4}$/
        query = query.code(q)
      else
        query = query.where("(name &@~ ?", q)
          .or(&.where_chong_wen_baos(
            cwb_query.where("chong_wen_baos.university_remark &@~ ?)", q),
            auto_inner_join: false
          ).left_join_chong_wen_baos)
      end
    end

    query = query.is_985(true) if is_985
    
    cwb_query = cwb_query.where do |wh|
      wh.is_excluded(false).or(&.is_excluded.is_nil)
    end

    cwb_query = cwb_query.university_remark.is_not_nil if is_exists_remark

    if is_marked ||
       chong_2023 || chong_2022
       wen_2023 || wen_2022
       bao_2023 || bao_2022
      cwb_query = cwb_query.is_marked(true) if is_marked

      if cwb_union_set
        cwb_query = cwb_query.where do |q|
          q = q.none
          q = chong_2023 ? q.or(&.chong_2023(true)) : q
          q = chong_2022 ? q.or(&.chong_2022(true)) : q
          
          q = wen_2023 ? q.or(&.wen_2023(true)) : q
          q = wen_2022 ? q.or(&.wen_2022(true)) : q

          q = bao_2023 ? q.or(&.bao_2023(true)) : q
          q = bao_2022 ? q.or(&.bao_2022(true)) : q

          q
        end
      else
        cwb_query = cwb_query.chong_2023(true) if chong_2023
        cwb_query = cwb_query.chong_2022(true) if chong_2022

        cwb_query = cwb_query.wen_2023(true) if wen_2023
        cwb_query = cwb_query.wen_2022(true) if wen_2022

        cwb_query = cwb_query.bao_2023(true) if bao_2023
        cwb_query = cwb_query.bao_2022(true) if bao_2022
      end
    end

    # ...

    query = query.join("left join chong_wen_baos on universities.id = chong_wen_baos.university_id AND chong_wen_baos.user_id = '#{current_user.id}'").where_chong_wen_baos(cwb_query, auto_inner_join: false)

    pages, universities = paginate(query.id.desc_order.distinct, per_page: 50)

@jwoertink
Copy link
Member

I think you're on the right direction with #1049 but maybe this would work better similar to custom order. With custom ORDER BY you can do this

custom_order = Avram::OrderBy.new("EXTRACT(HOUR FROM created_at)", :asc)
UserQuery.new.order_by(custom_order)

We can do custom joins with

custom_join = Avram::Join::Left.new(from: :users, to: :messages, foreign_key: :receiver_id)
UserQuery.new.join(custom_join)

So I think all we need at this point is maybe a Avram::RawJoin

custom_join = Avram::RawJoin::Left.new("chong_wen_baos on universities.id = chong_wen_baos.university_id AND chong_wen_baos.user_id = ?", current_user.id)
UniversityQuery.new.join(custom_join)

or, maybe Avram::Join::Left could have a method to append extra query info?

custom_join = Avram::Join::Left.new(from: :chong_wen_baos, to: :universities, foreign_key: :university_id)
custom_join.and("chong_wen_baos = ?", current_user.id)

I'm not sure what the proper solution is here, but this is also not something I've ever needed. If you want to submit a PR with one of these options, we can give it a try. I would like to stay as close to using the Avram::Join class as much as possible.

@zw963
Copy link
Contributor Author

zw963 commented Jun 21, 2024

I would like to stay as close to using the Avram::Join class as much as possible.

Me too, Avram::RawJoin is what i want. I guess i can try create a PR anyway.

BTW: i prefer always specify the join explicitly instead of join implicitly in the where_???, although this is a breaking change, but, more clearly, and make the implementation logic simpler.

@jwoertink
Copy link
Member

I don't think we need a breaking change for this update, but submit your PR and we will work through it.

@zw963
Copy link
Contributor Author

zw963 commented Jun 21, 2024

I don't think we need a breaking change for this update, but submit your PR and we will work through it.

Agree, only a recommendation for the future. 😄

@zw963 zw963 linked a pull request Jun 22, 2024 that will close this issue
zw963 added a commit to zw963/avram that referenced this issue Jun 22, 2024
@zw963
Copy link
Contributor Author

zw963 commented Jun 22, 2024

Hi, @jwoertink , i create a initial version of PR #1050, please have a look.

It's seem like works on my laptop after a simple test, although, same issue as my previous hack still happen.

for example: the last line of following code not work which cause a PQ::PQError because the INNER JOIN statement is not generated by the where_chong_wen_baos(excluded_query).

excluded_query = ChongWenBaoQuery.new.user_id(current_user.id).is_excluded(true)
query = UniversityQuery.new.preload_chong_wen_baos(excluded_query)
query = query.where_chong_wen_baos(excluded_query)

I have to change from

query = query.where_chong_wen_baos(excluded_query)

Into

query = query
  .join("inner join chong_wen_baos on universities.id = chong_wen_baos.university_id")
  .where_chong_wen_baos(excluded_query)

To make sure the query is working.

I'm curious why this happened because I just added some new code but without making any changes to the existing code related to where _??? macro, could you please explain why this happen?


Another concerns is, i am not running spec because i am out, no network to pull docker image for now, i will try add it later when this feature is start to working.

@zw963
Copy link
Contributor Author

zw963 commented Jun 23, 2024

I am running spec, see many spec failed about generated join automatically when use where_???.

failed spec
crystal spec spec/avram/query_builder/merge_spec.cr:4 # Avram::QueryBuilder#merge merges the wheres and joins
crystal spec spec/avram/query_associations_spec.cr:61 # Query associations can query associations with inner_join specified
crystal spec spec/avram/query_associations_spec.cr:81 # Query associations can query associations with left_join specified
crystal spec spec/avram/query_associations_spec.cr:101 # Query associations can query associations with right_join specified
crystal spec spec/avram/query_associations_spec.cr:121 # Query associations can query associations with full_join specified
crystal spec spec/avram/query_associations_spec.cr:141 # Query associations can query associations on namespaced models
crystal spec spec/avram/queryable_spec.cr:1059 # Avram::Queryable(T) #join methods for associations inner join on belongs to
crystal spec spec/avram/queryable_spec.cr:1079 # Avram::Queryable(T) #join methods for associations inner join on has many
crystal spec spec/avram/queryable_spec.cr:1099 # Avram::Queryable(T) #join methods for associations multiple inner joins on has many through
crystal spec spec/avram/queryable_spec.cr:1122 # Avram::Queryable(T) #left_join methods for associations left join on belongs to
crystal spec spec/avram/queryable_spec.cr:1141 # Avram::Queryable(T) #left_join methods for associations left join on has many
crystal spec spec/avram/queryable_spec.cr:1160 # Avram::Queryable(T) #left_join methods for associations multiple left joins on has many through
crystal spec spec/avram/query_builder_spec.cr:4 # Avram::QueryBuilder ensures uniqueness for orders, and joins
crystal spec spec/avram/query_builder_spec.cr:246 # Avram::QueryBuilder can be joined
crystal spec spec/avram/query_builder_spec.cr:299 # Avram::QueryBuilder clone copies over all parts of a query
crystal spec spec/avram/associations_spec.cr:116 # Avram::Model uuid backed models belongs_to returns associated model when using 'table' and 'foreign_key'
crystal spec spec/avram/query_associations_spec.cr:38 # Query associations can query associations
crystal spec spec/avram/query_associations_spec.cr:150 # Query associations handles potential joins over the table queried
crystal spec spec/avram/query_associations_spec.cr:165 # Query associations handles duplicate joins
crystal spec spec/avram/queryable_spec.cr:1358 # Avram::Queryable(T) #update updates with joins
crystal spec spec/avram/queryable_spec.cr:1475 # Avram::Queryable(T) #clone clones joined queries
crystal spec spec/avram/queryable_spec.cr:1591 # Avram::Queryable(T) queries joining with has_one when you query from the belongs_to side returns a record
crystal spec spec/avram/queryable_spec.cr:1601 # Avram::Queryable(T) queries joining with has_one when you query from the has_one side returns a record

I guess the reason cause this is, the new created joins method override the old one, when the joins_sql method run, it use the last defined (raw version) joins, which cause this issue. (please check following screenshot)

image

I have no idea about how to fix that. 😄

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 a pull request may close this issue.

2 participants