#2301 avoid redundant clauses and joins in query_buildroots()
Merged 2 years ago by tkopecek. Opened 2 years ago by mikem.
mikem/koji redundant-joins  into  master

file modified
-6
@@ -5373,9 +5373,6 @@ 

      # run separate queries for picking smallest candidate set

      candidate_buildroot_ids = set()

      if rpmID is not None:

-         joins.insert(0, 'buildroot_listing ON buildroot.id = buildroot_listing.buildroot_id')

-         fields.append(('buildroot_listing.is_update', 'is_update'))

Do we want to drop this field? It is (probably) not used anywhere, but it still an API change.

-         clauses.append('buildroot_listing.rpm_id = %(rpmID)i')

          query = QueryProcessor(columns=['buildroot_id'], tables=['buildroot_listing'],

                                 clauses=['rpm_id = %(rpmID)i'], opts={'asList': True},

                                 values=locals())
@@ -5385,8 +5382,6 @@ 

              return []

  

      if archiveID is not None:

-         joins.insert(0, 'buildroot_archives ON buildroot.id = buildroot_archives.buildroot_id')

-         clauses.append('buildroot_archives.archive_id = %(archiveID)i')

          query = QueryProcessor(columns=['buildroot_id'], tables=['buildroot_archives'],

                                 clauses=['archive_id = %(archiveID)i'], opts={'asList': True},

                                 values=locals())
@@ -5399,7 +5394,6 @@ 

              return []

  

      if taskID is not None:

-         clauses.append('standard_buildroot.task_id = %(taskID)i')

          query = QueryProcessor(columns=['buildroot_id'], tables=['standard_buildroot'],

                                 clauses=['task_id = %(taskID)i'], opts={'asList': True},

                                 values=locals())

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

Do we want to drop this field? It is (probably) not used anywhere, but it still an API change.

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

2 years ago

Do we want to drop this field? It is (probably) not used anywhere, but it still an API change.

I talked about that a little in #2300. It is technically an api change. We don't rely on the field anywhere, and the polymorphism of the result is a little odd (the field is only present when querying by rpmID).

Also, I think most (all?) places in the code where we query buildroots by rpm_id we use listRPMs instead.

It seemed strange to have the changes from #2274 yet still have those joins. Seems to almost defeat the point. Am I thinking about it wrong?

Yes, I agree it should be there. I was a bit afraid just about dropping this field. :thumbsup:

Commit 6e50120 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago