Re: Re: FETCH FIRST clause WITH TIES option
От | Tomas Vondra |
---|---|
Тема | Re: Re: FETCH FIRST clause WITH TIES option |
Дата | |
Msg-id | 20190331001446.GA10804@development обсуждение исходный текст |
Ответ на | Re: Re: FETCH FIRST clause WITH TIES option (Tomas Vondra <tomas.vondra@2ndquadrant.com>) |
Ответы |
Re: Re: FETCH FIRST clause WITH TIES option
Re: Re: FETCH FIRST clause WITH TIES option |
Список | pgsql-hackers |
On Fri, Mar 29, 2019 at 01:56:48AM +0100, Tomas Vondra wrote: >On Tue, Mar 26, 2019 at 10:46:00AM +0300, Surafel Temesgen wrote: >>On Mon, Mar 25, 2019 at 11:56 AM David Steele <david@pgmasters.net> wrote: >> >>>This patch no longer passes testing so marked Waiting on Author. >>> >>> >>Thank you for informing. Fixed > >Thanks for the updated patch. I do have this on my list of patches that >I'd like to commit in this CF - likely tomorrow after one more round of >review, or so. > Hi, I got to look at the patch today, with the intent to commit, but sadly I ran into a couple of minor issues that I don't feel comfortable fixing on my own. Attached is a patch highlighling some of the places (0001 is your v7 patch, to keep the cfbot happy). 1) the docs documented this as ... [ ONLY | WITH TIES ] but that's wrong, because it implies those options are optional (i.e. the user may not specify anything). That's not the case, exactly one of those options needs to be specified, so it should have been ... { ONLY | WITH TIES } 2) The comment in ExecLimit() needs to be updated to explain that WITH TIES changes the behavior. 3) Minor code style issues (no space before * on comment lines, {} around single-line if statements, ...). 4) The ExecLimit() does this if (node->limitOption == WITH_TIES) ExecCopySlot(node->last_slot, slot); but I think we only really need to do that for the last tuple in the window, no? Would it be a useful optimization? 5) Two issues in _outLimit(). Firstly, when printing uniqCollations the code actually prints uniqOperators. Secondly, why does the code use these loops at all, instead of using WRITE_ATTRNUMBER_ARRAY and WRITE_OID_ARRAY, like other places? Perhaps there's an issue with empty arrays? I haven't tested this, but looking at the READ_ counterparts, I don't see why that would be the case. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
В списке pgsql-hackers по дате отправления: