Re: [Apache Bloodhound] #654: Rows in ticket query result set not sorted by column

[Apache Bloodhound <de...@bloodhound.apache.org>]

#654: Rows in ticket query result set not sorted by column
---------------------------+-----------------------------------
  Reporter:  olemis        |      Owner:  gjm
      Type:  defect        |     Status:  accepted
  Priority:  blocker       |  Milestone:  Release 8
 Component:  multiproduct  |    Version:
Resolution:                |   Keywords:  ticket query, sorting
---------------------------+-----------------------------------

Comment (by gjm):

 OK, I think I am finally getting to the bottom of this. The issue appears
 to be that in SQL you cannot rely on the ordering of a subquery to
 maintain order in the outer query.

 If we don't wish to adjust the sql within {{{trac.ticket.query.Query}}}
 then the only ways I can think of to fix this at the moment involve
 capturing and editing the sql produced by that module rather than wrapping
 changes around it - and if we are going to do that then we might as well
 do the job properly so I am currently looking at replacing the overriding
 {{{multiproduct.ticket.query.Query.execute}}} method with something a
 little simpler and overriding
 {{{multiproduct.ticket.query.Query.get_sql}}} like so:

 {{{#!python
     def get_sql(self, *args, **kwargs):
         sql, args = super(ProductQuery, self).get_sql(*args, **kwargs)
         parts = sql.split('\n')
         enum_match = re.compile(r"^\s*LEFT OUTER JOIN enum "
                                 r"AS (?P<type>\b\w+\b) ON \(")
         enum_replace = ("LEFT OUTER JOIN enum AS \g<type> ON "
                         "(\g<type>.product=t.product AND ")
         sql = '\n'.join(enum_match.sub(enum_replace, part) for part in
 parts)
         return sql, args

     def execute(self, *args, **kwargs):
         old_mp_schema_enabled = self.env._multiproduct_schema_enabled
         results = super(ProductQuery, self).execute(*args, **kwargs)
         self.env._multiproduct_schema_enabled = old_mp_schema_enabled
         return results
 }}}

 The regex might be a little over-the-top and there may well be better ways
 of attacking the problem, particularly as there is some reliance on a
 fairly specific string to search for but it seems worth reporting sooner
 rather than later.

-- 
Ticket URL: <https://issues.apache.org/bloodhound/ticket/654#comment:11>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound issue tracker

Re: [Apache Bloodhound] #654: Rows in ticket query result set not sorted by column

[Gary Martin <ga...@wandisco.com>]

On 02/01/14 20:30, Apache Bloodhound wrote:
> #654: Rows in ticket query result set not sorted by column
> ---------------------------+-----------------------------------
>   Reporter:  olemis        |      Owner:  gjm
>       Type:  defect        |     Status:  accepted
>   Priority:  blocker       |  Milestone:  Release 8
>  Component:  multiproduct  |    Version:
> Resolution:                |   Keywords:  ticket query, sorting
> ---------------------------+-----------------------------------
>
> Comment (by gjm):
>
>  OK, I think I am finally getting to the bottom of this. The issue appears
>  to be that in SQL you cannot rely on the ordering of a subquery to
>  maintain order in the outer query.
>
>  If we don't wish to adjust the sql within {{{trac.ticket.query.Query}}}
>  then the only ways I can think of to fix this at the moment involve
>  capturing and editing the sql produced by that module rather than wrapping
>  changes around it - and if we are going to do that then we might as well
>  do the job properly so I am currently looking at replacing the overriding
>  {{{multiproduct.ticket.query.Query.execute}}} method with something a
>  little simpler and overriding
>  {{{multiproduct.ticket.query.Query.get_sql}}} like so:
>
>  {{{#!python
>      def get_sql(self, *args, **kwargs):
>          sql, args = super(ProductQuery, self).get_sql(*args, **kwargs)
>          parts = sql.split('\n')
>          enum_match = re.compile(r"^\s*LEFT OUTER JOIN enum "
>                                  r"AS (?P<type>\b\w+\b) ON \(")
>          enum_replace = ("LEFT OUTER JOIN enum AS \g<type> ON "
>                          "(\g<type>.product=t.product AND ")
>          sql = '\n'.join(enum_match.sub(enum_replace, part) for part in
>  parts)
>          return sql, args
>
>      def execute(self, *args, **kwargs):
>          old_mp_schema_enabled = self.env._multiproduct_schema_enabled
>          results = super(ProductQuery, self).execute(*args, **kwargs)
>          self.env._multiproduct_schema_enabled = old_mp_schema_enabled
>          return results
>  }}}
>
>  The regex might be a little over-the-top and there may well be better ways
>  of attacking the problem, particularly as there is some reliance on a
>  fairly specific string to search for but it seems worth reporting sooner
>  rather than later.
>

I am hoping that I understood the problem and I am looking at the right
thing this time but it seemed to fit the facts. It is possible to
reproduce with the postgresql database. (That it doesn't seem to go
wrong for the sqlite backend should probably just be considered a matter
of luck.) It should be possible to spot this issue adding three tickets
where the priorities vary appropriately.

Another look at what I pasted in the ticket suggests I may have
accidentally dropped the
{{{#!python
self.env._multiproduct_schema_enabled = False
}}}

Part of what it currently does is to use that to temporarily switch to
using db_query instead of the db_direct_query method. I was hoping to be
able to temporarily add product constraints to the Query.contraints
object instead but my quick attempt at this failed.

I can't say that I am all that happy with the solution yet but it has
the virtue of removing a lot of unneeded code at least. Anyway, I'll add
or improve test cases soon.

Cheers,
    Gary