Good to know that I was not to far off target then. But fixing issues in less than a day of reporting? On a Saturday? Who does that? I was planning to feel happy about solving this issue.. :)
Fredrik On Sat, Sep 21, 2019 at 9:31 PM Dan Kennedy <danielk1...@gmail.com> wrote: > > On 22/9/62 02:25, Fredrik Larsen wrote: > > Interesting, very similar change but not fully idenctial. In my patch, I > > created a sqlite3ExprListCompareIgnoreButUpdateSort, and used this > function > > from line 6239. This function ignores the sort part when comparing > > expressions, but will update the GroupBy sortOrder field if expressions > are > > found equal. I see dan modifies the sortFlag. > > That sounds equivalent to me. The sortOrder/sortFlag thing is probably > just because you patched the last release (3.29.0) or earlier. The field > has changed names since then. > > Dan. > > > > > Would be interesting to know > > what effect this difference has. My change works in my test, but not sure > > if it really works. > > > > Anyway, super-nice that this issue is fixed officially. No I don't have > to > > wonder if my fix is really correct, or will suddenly corrupt something :) > > > > Fredrik > > > > On Sat, Sep 21, 2019 at 8:49 PM Keith Medcalf <kmedc...@dessus.com> > wrote: > > > >> See Dan's checkin on trunk for this issue. > >> > >> https://www.sqlite.org/src/info/20f7951bb238ddc0 > >> > >>> -----Original Message----- > >>> From: sqlite-users <sqlite-users-boun...@mailinglists.sqlite.org> On > >>> Behalf Of Fredrik Larsen > >>> Sent: Saturday, 21 September, 2019 08:12 > >>> To: SQLite mailing list <sqlite-users@mailinglists.sqlite.org> > >>> Subject: Re: [sqlite] [EXTERNAL] Group-by and order-by-desc does not > work > >>> as expected > >>> > >>> Your last sentence got me thinking. So I downloaded the source, > modified > >>> the ordering of the GROUP-BY expression to match ORDER-BY and it works! > >>> This will offcourse only work if the GROUP-BY and ORDER-BY matches > >>> generally expect for the direction. This fix only improves performance > >>> for > >>> relevant cases and keeps other cases unaffected. Not sure if I > introduced > >>> some subtle bugs with this modification, but my test-cases runs fine. > >>> > >>> Fredrik > >>> > >>> On Fri, Sep 20, 2019 at 6:57 PM Keith Medcalf <kmedc...@dessus.com> > >>> wrote: > >>> > >>>>> We can observe GROUP BY works ASCending only as of now. Why it can't > >>> work > >>>>> DESCending to avoid ordering, that's a different question. > >>>> >From https://www.sqlite.org/lang_select.html we can observe that > >>>>> GROUP BY takes an expr on the RHS, while ORDER BY takes an expr > >>>>> followed by optional COLLATE and ASC/DESC terms. > >>>> The GROUP BY clause does not imply ordering. The fact that the output > >>> is > >>>> ordered is an implementation detail -- the grouping could be > >>> implemented by > >>>> a hash table, in which case the output would be ordered by hash value, > >>> for > >>>> instance. All that the expression in a GROUP BY does is determine the > >>>> groupings, and therefore the expression is limited to a comparison > >>>> compatible expression. For example, you can GROUP BY x COLLATE NOCASE > >>>> which implies that the groups are formed using case insensitive > >>> comparisons > >>>> of x. The ORDER BY clause determines the output ordering. > >>>> > >>>> You will note that if you do the following: > >>>> > >>>> create table x(x,y); > >>>> create index ix on x(x desc, y); > >>>> select x, someaggregate(y) from x group by x order by x desc; > >>>> > >>>> then ix will be used as a covering index (which is good) however the > >>> group > >>>> by x is treated as an ordering expression, not as simply a grouping > >>>> expression. > >>>> > >>>> In fact the code that implements the group by does indeed (perhaps > >>>> erroneously) treat the group by expression as implying order, since it > >>> will > >>>> traverse the covering index in reverse order so that the output from > >>> GROUP > >>>> BY is in ascending order, and add an extra sort to do the ORDER BY. > >>>> > >>>> That means the GROUP BY code generator is already capable of > traversing > >>>> the selected index in reverse order when necessary. It appears that > >>> the > >>>> optimizer however does not recognize that the "desc" attribute from > the > >>>> order by can be "pushed down" into the GROUP BY (which really is > >>> ordering > >>>> as an implementation detail) thus eliminating the ORDER BY processing > >>>> entirely. > >>>> > >>>> Note that you cannot specify that the GROUP BY is ordering -- it will > >>> not > >>>> accept the ASC or DESC keywords (which is correct), and this should > not > >>> be > >>>> changed, however, treating it as being ordering when it is not might > >>>> perhaps be a defect ... > >>>> > >>>> > >>>> > >>>> _______________________________________________ > >>>> sqlite-users mailing list > >>>> sqlite-users@mailinglists.sqlite.org > >>>> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users > >>>> > >>> _______________________________________________ > >>> sqlite-users mailing list > >>> sqlite-users@mailinglists.sqlite.org > >>> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users > >> > >> > >> _______________________________________________ > >> sqlite-users mailing list > >> sqlite-users@mailinglists.sqlite.org > >> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users > >> > > _______________________________________________ > > sqlite-users mailing list > > sqlite-users@mailinglists.sqlite.org > > http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users > _______________________________________________ > sqlite-users mailing list > sqlite-users@mailinglists.sqlite.org > http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users > _______________________________________________ sqlite-users mailing list sqlite-users@mailinglists.sqlite.org http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
