On Jan 16, 11:41 pm, osimons <[email protected]> wrote:
> On Jan 16, 11:24 pm, osimons <[email protected]> wrote:
>
> > On Jan 16, 10:46 pm, Christian Boos <[email protected]> wrote:
> > > On 1/16/2012 12:17 AM, osimons wrote:
> > > > 2) Should we try to parse the report SQL and somehow extract any
> > > > underlying order specification?
>
> > > It seems it could be "enough" to check if the SQL query is ending
> > > with an `ORDER BY`, and in that case simply insert our extra
> > > order columns, if any. But as that clause may contain arbitrary
> > > expressions, we should at least check for an even count of
> > > parentheses between the ORDER BY and the end of the query.
>
> > If we can safely extract and reuse the ORDER BY clause of the
> > underlying query, we have no additional columns to insert - we would
> > just reuse that at top level.
>
> I just covered normal report execution when I said we could reuse it
> as-is. However, if the user clicks on a column to re-sort the report
> manually (once to select a new sort column ASC, doing it twice to
> switch it to DESC) we would need to completely replace the ORDER BY
> inside the report and replace it with "ORDER BY [__group__, ] <field>
> ASC|DESC" (__group__ only used if the report is grouped of course).
I've spent some time thinking about this, and also had helpful
discussions with 'CareBear\' and 'shesek' today on #trac IRC.
The following is a summary of status and possible approaches:
1. REVERT TO INITIAL HANDLING
-----------------------------
Revert the patch, and leave it at that. Best described as the 2
fundamental issues apparent here:
a) It works, mostly. But in essence it returns rows in undefined
order, and we are depending on undefined order to paginate and render
results. Issues like groups appearing several times in reports cannot
be avoided. Rows may appear out of 'order'.
b) The order of groups as now reported as a 'regression' remains, but
only when you re-sort the report. Like our own /demo-0.12/report/8
("Active Tickets, Mine first") do this test;
- open report, observe that it says "My Tickets"
- click any column header to resort table (like "Component")
- observe that it now says "Active Tickets" as first group.
All r10892 did was make this bug apparent as without parsing the query
we can't know if the order we insert is supposed to be ASC or DESC.
After r10892 the bug will be clearly visible for all reports using
ORDER BY __group__ DESC.
2. DROP SUBQUERY AND PAGINATE IN PYTHON
---------------------------------------
Instead of LIMIT and OFFSET in SQL, return the full resultset and pick
the rows we want depending on paging status.
a) The sorting would be correct. But, it will come at the cost of
performance as the cursor will be populated with full data sets for
each page in the report. The performance must be measured and compared
for larger datasets as the penalty may be very noticeable.
b) Switching to Python paging does not help with detecting ASC or DESC
sorting of groups. Issue remains.
3. USE ROW_NUMBER IN SQL
------------------------
Supported for MySQL and Postgres, we could mofify the inner SQL to be
"SELECT ROW_NUMBER(), ...<remaing query>..." to have an ID to follow
each row that we then could reuse at outer level.
a) The sorting would be correct, but no solution in SQLite that does
not support it. Would need conditional code that uses initial
behaviour for SQLite, and other behaviour for those that are known to
support it. There should be no noticeable performance differences
compared to current status.
b) Again, no new knowledge about __group__ ASC or DESC.
4. PARSE QUERY AND EXTRACT ORDER
--------------------------------
Somehow parse the query and extract the details we need, really just
the ORDER BY clause content. If __group__ are part of current columns,
we would need to split the ORDER BY content into parts (can order by
more than one item of course) and detect if DESC is mentioned for
first order item. As the SQL in theory can contain further nested
subqueries, and "ORDER BY" text may otherwise appear in functions or
elsewhere, care must be taken to extract this correctly.
a) By appending the subquery ORDER BY to the outer query, the results
will be ordered correctly.
b) With the contents of the ORDER BY clause in hand, parsed and
correctly identified ASC/DESC of first item, we can also re-sort
grouped reports by doing "ORDER BY __group__ ASC|DESC, selected_column
ASC|DESC". The outer and inner order should also be correct.
5. JUST APPEND LIMIT AND OFFSET TO THE QUERY
--------------------------------------------
As Christian suggested, just drop the use of subquery and append
"LIMIT ... OFFSET .." to the original query.
a) The order would be correct. New issue may be that the report would
fail if the user-entered SQL already contains the same keywords.
Typically reports that contain the "top number of.." or the "most
recent.." that itself restricts the dataset. It would be a bug, and we
would need a fallback strategy - like for instance just use current
strategy if exception occurs (1.).
b) Re-sorting columns would not be possible seeing we would not be
able to change the ORDER BY clause. This would of be much worse than
the current/initial situation.
6. "YOUR" SUGGESTION?
---------------------
Feel free to suggest other approaches - or combination of approaches.
:::simon
https://www.coderesort.com
http://trac-hacks.org/wiki/osimons
--
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en.
