Thread: pgsql: Support FETCH FIRST WITH TIES
Support FETCH FIRST WITH TIES WITH TIES is an option to the FETCH FIRST N ROWS clause (the SQL standard's spelling of LIMIT), where you additionally get rows that compare equal to the last of those N rows by the columns in the mandatory ORDER BY clause. There was a proposal by Andrew Gierth to implement this functionality in a more powerful way that would yield more features, but the other patch had not been finished at this time, so we decided to use this one for now in the spirit of incremental development. Author: Surafel Temesgen <surafel3000@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tomas Vondra <tomas.vondra@2ndquadrant.com> Discussion: https://postgr.es/m/CALAY4q9ky7rD_A4vf=FVQvCGngm3LOes-ky0J6euMrg=_Se+ag@mail.gmail.com Discussion: https://postgr.es/m/87o8wvz253.fsf@news-spur.riddles.org.uk Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/357889eb17bb9c9336c4f324ceb1651da616fe57 Modified Files -------------- doc/src/sgml/ref/select.sgml | 15 +-- src/backend/catalog/sql_features.txt | 2 +- src/backend/executor/nodeLimit.c | 160 +++++++++++++++++++++++++++--- src/backend/nodes/copyfuncs.c | 7 ++ src/backend/nodes/equalfuncs.c | 2 + src/backend/nodes/outfuncs.c | 7 ++ src/backend/nodes/readfuncs.c | 6 ++ src/backend/optimizer/plan/createplan.c | 45 ++++++++- src/backend/optimizer/plan/planner.c | 1 + src/backend/optimizer/util/pathnode.c | 2 + src/backend/parser/analyze.c | 21 ++-- src/backend/parser/gram.y | 117 +++++++++++++++++----- src/backend/parser/parse_clause.c | 15 ++- src/backend/utils/adt/ruleutils.c | 27 ++++-- src/include/catalog/catversion.h | 2 +- src/include/nodes/execnodes.h | 5 + src/include/nodes/nodes.h | 13 +++ src/include/nodes/parsenodes.h | 2 + src/include/nodes/pathnodes.h | 1 + src/include/nodes/plannodes.h | 5 + src/include/optimizer/pathnode.h | 1 + src/include/optimizer/planmain.h | 5 +- src/include/parser/parse_clause.h | 3 +- src/test/regress/expected/limit.out | 167 ++++++++++++++++++++++++++++++++ src/test/regress/sql/limit.sql | 48 +++++++++ 25 files changed, 610 insertions(+), 69 deletions(-)
Hi Álvaro, On Tue, Apr 7, 2020 at 10:28 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > Support FETCH FIRST WITH TIES > > WITH TIES is an option to the FETCH FIRST N ROWS clause (the SQL > standard's spelling of LIMIT), where you additionally get rows that > compare equal to the last of those N rows by the columns in the > mandatory ORDER BY clause. > > There was a proposal by Andrew Gierth to implement this functionality in > a more powerful way that would yield more features, but the other patch > had not been finished at this time, so we decided to use this one for > now in the spirit of incremental development. > > Author: Surafel Temesgen <surafel3000@gmail.com> > Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> > Reviewed-by: Tomas Vondra <tomas.vondra@2ndquadrant.com> > Discussion: https://postgr.es/m/CALAY4q9ky7rD_A4vf=FVQvCGngm3LOes-ky0J6euMrg=_Se+ag@mail.gmail.com > Discussion: https://postgr.es/m/87o8wvz253.fsf@news-spur.riddles.org.uk > > Branch > ------ > master > > Details > ------- > https://git.postgresql.org/pg/commitdiff/357889eb17bb9c9336c4f324ceb1651da616fe57 > > Modified Files > -------------- > doc/src/sgml/ref/select.sgml | 15 +-- > src/backend/catalog/sql_features.txt | 2 +- > src/backend/executor/nodeLimit.c | 160 +++++++++++++++++++++++++++--- > src/backend/nodes/copyfuncs.c | 7 ++ > src/backend/nodes/equalfuncs.c | 2 + > src/backend/nodes/outfuncs.c | 7 ++ > src/backend/nodes/readfuncs.c | 6 ++ > src/backend/optimizer/plan/createplan.c | 45 ++++++++- > src/backend/optimizer/plan/planner.c | 1 + > src/backend/optimizer/util/pathnode.c | 2 + > src/backend/parser/analyze.c | 21 ++-- > src/backend/parser/gram.y | 117 +++++++++++++++++----- > src/backend/parser/parse_clause.c | 15 ++- > src/backend/utils/adt/ruleutils.c | 27 ++++-- > src/include/catalog/catversion.h | 2 +- > src/include/nodes/execnodes.h | 5 + > src/include/nodes/nodes.h | 13 +++ > src/include/nodes/parsenodes.h | 2 + > src/include/nodes/pathnodes.h | 1 + > src/include/nodes/plannodes.h | 5 + > src/include/optimizer/pathnode.h | 1 + > src/include/optimizer/planmain.h | 5 +- > src/include/parser/parse_clause.h | 3 +- > src/test/regress/expected/limit.out | 167 ++++++++++++++++++++++++++++++++ > src/test/regress/sql/limit.sql | 48 +++++++++ > 25 files changed, 610 insertions(+), 69 deletions(-) > FTR I now get the following when compiling with -Wimplicit-fallthrough -Werror: nodeLimit.c: In function ‘ExecLimit’: nodeLimit.c:136:7: error: this statement may fall through [-Werror=implicit-fallthrough=] 136 | if (ScanDirectionIsForward(direction)) | ^ nodeLimit.c:216:3: note: here 216 | case LIMIT_WINDOWEND_TIES: | ^~~~ It seems that this fall-through comment: [...] else { node->lstate = LIMIT_WINDOWEND_TIES; /* fall-through */ } [...] Is not recognized by my compiler (gcc (Gentoo 9.3.0 p1) 9.3.0). If that's something we should fix, PFA a naive patch for that.
Attachment
Julien Rouhaud <rjuju123@gmail.com> writes:
> On Tue, Apr 7, 2020 at 10:28 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> Support FETCH FIRST WITH TIES
> FTR I now get the following when compiling with -Wimplicit-fallthrough -Werror:
Yeah, assorted buildfarm animals are mentioning that too. I wonder
if we should add that to the default warning options selected by
configure? I don't remember if that's been discussed before.
> It seems that this fall-through comment:
> /* fall-through */
> Is not recognized by my compiler (gcc (Gentoo 9.3.0 p1) 9.3.0). If
> that's something we should fix, PFA a naive patch for that.
Hmm, I feel like this logic is baroque enough to need more of a rewrite
than that :-(. But not sure exactly what would be better, so your
patch seems reasonable for now. The comments could use some help too.
regards, tom lane