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. 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
