Re: FETCH FIRST clause WITH TIES option

On 2020-Apr-08, Andrew Gierth wrote: > > "Alvaro" == Alvaro Herrera writes: > > >> This needs to be fixed in ruleutils, IMO, not by changing what the > >> grammar accepts. > > Alvaro> Fair. I didn't change what the grammar accepts. I ended up only > Alvaro> throwing an error in parse

Re: FETCH FIRST clause WITH TIES option

> "Alvaro" == Alvaro Herrera writes: >> This needs to be fixed in ruleutils, IMO, not by changing what the >> grammar accepts. Alvaro> Fair. I didn't change what the grammar accepts. I ended up only Alvaro> throwing an error in parse analysis when a bare NULL const is Alvaro> seen.

Re: FETCH FIRST clause WITH TIES option

On 2020-Apr-08, Andrew Gierth wrote: > > "Alvaro" == Alvaro Herrera writes: > > Alvaro> It turns out that the SQL standard is much more limited in what > Alvaro> it will accept there. But our grammar (what we'll accept for > Alvaro> the ancient LIMIT clause) is very lenient -- it'll take

Re: FETCH FIRST clause WITH TIES option

> "Alvaro" == Alvaro Herrera writes: Alvaro> It turns out that the SQL standard is much more limited in what Alvaro> it will accept there. But our grammar (what we'll accept for Alvaro> the ancient LIMIT clause) is very lenient -- it'll take just Alvaro> any expression. I thought about

Re: FETCH FIRST clause WITH TIES option

Hello On 2020-Apr-07, Andres Freund wrote: > On 2020-04-07 16:36:54 -0400, Alvaro Herrera wrote: > > Pushed, with some additional changes. > > This triggers a new warning for me (gcc-10): > /home/andres/src/postgresql/src/backend/executor/nodeLimit.c: In function > ‘ExecLimit’: >

Re: FETCH FIRST clause WITH TIES option

Hi, On 2020-04-07 16:36:54 -0400, Alvaro Herrera wrote: > Pushed, with some additional changes. This triggers a new warning for me (gcc-10): /home/andres/src/postgresql/src/backend/executor/nodeLimit.c: In function ‘ExecLimit’: /home/andres/src/postgresql/src/backend/executor/nodeLimit.c:136:7:

Re: FETCH FIRST clause WITH TIES option

Pushed, with some additional changes. So *of course* when I add tests to verify that ruleutils, I find a case that does not work properly -- meaning, you get a view that pg_dump emits in a way that won't be accepted: CREATE VIEW limit_thousand_v_3 AS SELECT thousand FROM onek WHERE thousand <

Re: FETCH FIRST clause WITH TIES option

Some ruleutils.c code added by this patch is not covered by tests: 5246 : /* Add the LIMIT clause if given */ 52471115 : if (query->limitOffset != NULL) 5248 : { 5249 0 : appendContextKeyword(context, " OFFSET ",

Re: FETCH FIRST clause WITH TIES option

7 FETCH FIRST clause: WITH TIES option NO +F867 FETCH FIRST clause: WITH TIES option YES R010 Row pattern recognition: FROM clause NO R020 Row pattern recognition: WINDOW clause NO R030 Row pattern recognition: full aggregate support NO diff --git a/src/backend/executor/nodeLimit.

Re: FETCH FIRST clause WITH TIES option

ows either order. diff --git a/src/backend/catalog/sql_features.txt b/src/backend/catalog/sql_features.txt index 9f840ddfd2..746c9a2516 100644 --- a/src/backend/catalog/sql_features.txt +++ b/src/backend/catalog/sql_features.txt @@ -345,7 +345,7 @@ F863 Nested in YES F864 Top-level in views YE

Re: FETCH FIRST clause WITH TIES option

On Fri, Nov 29, 2019 at 05:19:00AM +, Andrew Gierth wrote: "Alvaro" == Alvaro Herrera writes: Alvaro> First, I noticed that there's a significant unanswered issue Alvaro> from Andrew Gierth about this using a completely different Alvaro> mechanism, namely an implicit window function.

Re: FETCH FIRST clause WITH TIES option

> "Alvaro" == Alvaro Herrera writes: Alvaro> First, I noticed that there's a significant unanswered issue Alvaro> from Andrew Gierth about this using a completely different Alvaro> mechanism, namely an implicit window function. Robert was Alvaro> concerned about the performance of

Re: FETCH FIRST clause WITH TIES option

On 2019-Nov-28, Surafel Temesgen wrote: > On Thu, Nov 28, 2019 at 12:36 AM Alvaro Herrera > wrote: > > > I think you should add a /* fall-though */ comment after changing state. > > Like this (this flow seems clearer; also DRY): > > > > if (!node->noCount && > >

Re: FETCH FIRST clause WITH TIES option

9 > (4 filas) > > This is just crazy. > > I think you need to stare a that thing a little harder. > > This is because the new state didn't know about backward scan and there was off by one error it scan one position past to detect ties. The attached patch fix both regards Sur

Re: FETCH FIRST clause WITH TIES option

Thanks. (I would suggest renaming the new state LIMIT_WINDOWEND_TIES, because it seems to convey what it does a little better.) I think you should add a /* fall-though */ comment after changing state. Like this (this flow seems clearer; also DRY): if (!node->noCount &&

Re: FETCH FIRST clause WITH TIES option

res.txt +++ b/src/backend/catalog/sql_features.txt @@ -345,7 +345,7 @@ F863 Nested in YES F864 Top-level in views YES F865 in YES F866 FETCH FIRST clause: PERCENT option NO -F867 FETCH FIRST clause: WITH TIES option NO +F867 FETCH FIRST clause: WITH TIES option YES R010 Row

Re: FETCH FIRST clause WITH TIES option

catalog/sql_features.txt +++ b/src/backend/catalog/sql_features.txt @@ -345,7 +345,7 @@ F863 Nested in YES F864 Top-level in views YES F865 in YES F866 FETCH FIRST clause: PERCENT option NO -F867 FETCH FIRST clause: WITH TIES option NO +F867 FETCH FIRST clause: WITH TIES option YES

Re: FETCH FIRST clause WITH TIES option

(Prior to posting this delta patch, the CF bot appeared happy with this patch.) -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: FETCH FIRST clause WITH TIES option

On 2019-Nov-11, Alvaro Herrera wrote: > I'm not sure the proposed changes to gram.y are all that great, though. Here's a proposed simplification of the gram.y changes. There are two things here: 1. cosmetic: we don't need the LimitClause struct; we can use just SelectLimit, and return that

Re: FETCH FIRST clause WITH TIES option

On Mon, Nov 11, 2019 at 5:56 PM Alvaro Herrera wrote: > First, I noticed that there's a significant unanswered issue from Andrew > Gierth about this using a completely different mechanism, namely an > implicit window function. I see that Robert was concerned about the > performance of Andrew's

Re: FETCH FIRST clause WITH TIES option

(BTW another small issue in the patch is the comments in gram.y that talk about ONLY being a reserved word about the SQL:2008 syntax; ISTM that that comment should now mention both ONLY as well as WITH being fully reserved words.) -- Álvaro Herrerahttps://www.2ndQuadrant.com/

Re: FETCH FIRST clause WITH TIES option

sted in YES F864 Top-level in views YES F865 in YES F866 FETCH FIRST clause: PERCENT option NO -F867 FETCH FIRST clause: WITH TIES option NO +F867 FETCH FIRST clause: WITH TIES option YES R010 Row pattern recognition: FROM clause NO R020 Row pattern recognition: WIND

Re: FETCH FIRST clause WITH TIES option

Alvaro Herrera writes: > create table w (a point); > # select * from w order by a fetch first 2 rows with ties; > ERROR: could not identify an ordering operator for type point > LINE 1: select * from w order by a fetch first 2 rows with ties; > ^ > HINT: Use

Re: FETCH FIRST clause WITH TIES option

Hmm, create table w (a point); # select * from w order by a fetch first 2 rows with ties; ERROR: could not identify an ordering operator for type point LINE 1: select * from w order by a fetch first 2 rows with ties; ^ HINT: Use an explicit ordering operator

Re: FETCH FIRST clause WITH TIES option

.txt b/src/backend/catalog/sql_features.txt index 059ec02cd0..54628657e6 100644 --- a/src/backend/catalog/sql_features.txt +++ b/src/backend/catalog/sql_features.txt @@ -349,7 +349,7 @@ F863 Nested in YES F864 Top-level in views YES F865 in YES F866 FETCH FIRST clause: PERCENT option

Re: FETCH FIRST clause WITH TIES option

On 2019-Sep-06, Surafel Temesgen wrote: > > ... yet this doesn't appear to have resulted in any change in the code, > > or I just missed it. Are you going to update the patch per that? > > Its already done in v9 of the patch attached by Tomas Oh, I missed that. Great, so we're expecting some

Re: FETCH FIRST clause WITH TIES option

Hi Alvaro, On Fri, Sep 6, 2019 at 1:52 AM Alvaro Herrera from 2ndQuadrant < alvhe...@alvh.no-ip.org> wrote: > As Tom just said in the thread for PERCENT, the gram.y changes need a > better representation. Also, rename EXACT_NUMBER, per that thread. > > As far as I can tell, this concerns feature

Re: FETCH FIRST clause WITH TIES option

As Tom just said in the thread for PERCENT, the gram.y changes need a better representation. Also, rename EXACT_NUMBER, per that thread. As far as I can tell, this concerns feature F867. I think we should mark that as supported after this patch -- please edit

Re: FETCH FIRST clause WITH TIES option

Hi Erik, Thank you for looking at it. Can you have a look? > > It is because of the patch didn't handle that edge case. attache patch contain a fix for it regards Surafel diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 06d611b64c..e83d309c5b 100644 ---

Re: FETCH FIRST clause WITH TIES option

On 2019-07-01 19:38, Surafel Temesgen wrote: Thank you for informing. attach is a rebased patch against current master [...] [fetch_first_with_ties_v10.patch] Hi Surafel, The patch applies OK, make check is OK, compiles OK. But I get: TRAP: FailedAssertion("!(!(((slot)->tts_flags & (1 <<

Re: FETCH FIRST clause WITH TIES option

Hi Thomas On Mon, Jul 1, 2019 at 1:31 PM Thomas Munro wrote: > On Mon, Apr 8, 2019 at 8:26 PM Surafel Temesgen > wrote: > > agree > > Hi Surafel, > > A new Commitfest is starting. Can you please post a fresh rebase of this > patch? > > Thank you for informing. attach is a rebased patch against

Re: FETCH FIRST clause WITH TIES option

On Mon, Apr 8, 2019 at 8:26 PM Surafel Temesgen wrote: > agree Hi Surafel, A new Commitfest is starting. Can you please post a fresh rebase of this patch? Thanks, -- Thomas Munro https://enterprisedb.com

Re: FETCH FIRST clause WITH TIES option

On Sun, Apr 7, 2019 at 1:39 AM Tomas Vondra wrote: > > 1) I've removed the costing changes. As Tom mentions elsewhere in this > thread, that's probably not needed for v1 and it's true those estimates > are probably somewhat sketchy anyway. > > > 2) v8 did this (per my comment): > > -

Re: FETCH FIRST clause WITH TIES option

Hi Surafel, I've been looking at the patch with the intent to commit it, but once again I ran into stuff that seems suspicious to me but am not familiar with enough to say if it's OK. I'm sorry about that :-( First, a couple of tweaks in the attached v9 of the patch: 1) I've removed the

Re: FETCH FIRST clause WITH TIES option

On Wed, Apr 03, 2019 at 03:08:05PM -0400, Tom Lane wrote: Tomas Vondra writes: I've tried to fix the merge conflict (essentially by moving some of the code to adjust_limit_rows_costs(), but I'm wondering if the code added to create_limit_path is actually correct ... Firstly, this seriously

Re: FETCH FIRST clause WITH TIES option

Tomas Vondra writes: > I've tried to fix the merge conflict (essentially by moving some of the > code to adjust_limit_rows_costs(), but I'm wondering if the code added to > create_limit_path is actually correct > ... > Firstly, this seriously needs some comment explaining why we do this. I've

Re: FETCH FIRST clause WITH TIES option

Hi, Unfortunately this got broken again, this time by aef65db676 :-( I've tried to fix the merge conflict (essentially by moving some of the code to adjust_limit_rows_costs(), but I'm wondering if the code added to create_limit_path is actually correct if (count_est != 0) { double

Re: Re: FETCH FIRST clause WITH TIES option

On Sun, Mar 31, 2019 at 3:14 AM Tomas Vondra wrote: > > > 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

Re: Re: FETCH FIRST clause WITH TIES option

On Sun, Mar 31, 2019 at 01:14:46AM +0100, Tomas Vondra wrote: 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 wrote: This patch no longer passes testing so marked

Re: Re: FETCH FIRST clause WITH TIES option

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 wrote: This patch no longer passes testing so marked Waiting on Author. Thank you for informing. Fixed Thanks

Re: Re: FETCH FIRST clause WITH TIES option

On Tue, Mar 26, 2019 at 10:46:00AM +0300, Surafel Temesgen wrote: On Mon, Mar 25, 2019 at 11:56 AM David Steele 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

Re: Re: FETCH FIRST clause WITH TIES option

On Mon, Mar 25, 2019 at 11:56 AM David Steele wrote: > This patch no longer passes testing so marked Waiting on Author. > > Thank you for informing. Fixed diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 06d611b64c..b3b045ea87 100644 ---

Re: Re: FETCH FIRST clause WITH TIES option

On 2/4/19 2:37 PM, Surafel Temesgen wrote: On Mon, Feb 4, 2019 at 8:29 AM Michael Paquier > wrote: This patch needs a rebase because of conflicts done recently for pluggable storage, so moved to next CF, waiting on author. -- Thank you . rebased

Re: FETCH FIRST clause WITH TIES option

On Mon, Feb 4, 2019 at 8:29 AM Michael Paquier wrote: > > This patch needs a rebase because of conflicts done recently for > pluggable storage, so moved to next CF, waiting on author. > -- > Thank you . rebased against current master regards Surafel diff --git a/doc/src/sgml/ref/select.sgml

Re: FETCH FIRST clause WITH TIES option

On Wed, Jan 16, 2019 at 11:45:46AM +0300, Surafel Temesgen wrote: > The attached patch use your suggestion uptread This patch needs a rebase because of conflicts done recently for pluggable storage, so moved to next CF, waiting on author. -- Michael signature.asc Description: PGP signature

Re: FETCH FIRST clause WITH TIES option

On Tue, Jan 15, 2019 at 2:51 PM Tomas Vondra wrote: > What do you mean by "raw statistic"? > I mean without further calculation to consider other operation > > IMHO the best thing you can do is call estimate_num_groups() and combine > that with the number of input rows. That shall benefit

Re: FETCH FIRST clause WITH TIES option

On 1/15/19 11:07 AM, Surafel Temesgen wrote: > > > On Wed, Jan 2, 2019 at 6:19 PM Tomas Vondra > mailto:tomas.von...@2ndquadrant.com>> wrote: > > After looking at the "FETCH FIRST ... PERCENT" patch, I wonder if this > patch should tweak estimates in some way. Currently, the

Re: FETCH FIRST clause WITH TIES option

On Wed, Jan 2, 2019 at 6:19 PM Tomas Vondra wrote: > After looking at the "FETCH FIRST ... PERCENT" patch, I wonder if this > patch should tweak estimates in some way. Currently, the cardinality > estimate is the same as for plain LIMIT, using the requested number of > rows. But let's say there

Re: FETCH FIRST clause WITH TIES option

On 1/2/19 11:51 AM, Surafel Temesgen wrote: > > > On Tue, Jan 1, 2019 at 8:38 PM Tomas Vondra > mailto:tomas.von...@2ndquadrant.com>> wrote: > > > > >     The attached patch include all the comment given by Tomas and i > >     check sql standard about LIMIT and this feature >

Re: FETCH FIRST clause WITH TIES option

On Tue, Jan 1, 2019 at 8:38 PM Tomas Vondra wrote: > > > > The attached patch include all the comment given by Tomas and i > > check sql standard about LIMIT and this feature > > > > Unfortunately, it seems the "limit" regression tests fail for some > reason - the output mismatches the

Re: FETCH FIRST clause WITH TIES option

Hi, On 11/24/18 10:28 AM, Surafel Temesgen wrote: > Attach is rebased patch against the current master > regards > Surafel > > On Thu, Nov 1, 2018 at 2:28 PM Surafel Temesgen > wrote: > > hi, > > The attached patch include all the comment given by Tomas

Re: FETCH FIRST clause WITH TIES option

Attach is rebased patch against the current master regards Surafel On Thu, Nov 1, 2018 at 2:28 PM Surafel Temesgen wrote: > hi, > > The attached patch include all the comment given by Tomas and i check sql > standard about LIMIT and this feature > > it did not say anything about it but I think

Re: FETCH FIRST clause WITH TIES option

hi, The attached patch include all the comment given by Tomas and i check sql standard about LIMIT and this feature it did not say anything about it but I think its good idea to include it to LIMIT too and I will add it if we have consensus on it. regards surafel diff --git

Re: FETCH FIRST clause WITH TIES option

On 10/31/2018 06:17 PM, Robert Haas wrote: On Mon, Oct 29, 2018 at 12:48 PM Andrew Gierth wrote: Then FETCH FIRST N WITH TIES becomes "stop when the expression rank() over (order by ...) <= N is no longer true" (where the ... is the existing top level order by) Wouldn't that be wicked

Re: FETCH FIRST clause WITH TIES option

On Mon, Oct 29, 2018 at 12:48 PM Andrew Gierth wrote: > Then FETCH FIRST N WITH TIES becomes "stop when the expression > rank() over (order by ...) <= N > is no longer true" (where the ... is the existing top level order by) Wouldn't that be wicked expensive compared to something hard-coded

Re: FETCH FIRST clause WITH TIES option

On 10/29/2018 05:47 PM, Andrew Gierth wrote: "Tomas" == Tomas Vondra writes: >> I still think that this is the wrong approach. Implementing WITH >> TIES and PERCENT together using an implicit window function call >> kills two birds with one very small stone (the only executor change

Re: FETCH FIRST clause WITH TIES option

> "Tomas" == Tomas Vondra writes: >> I still think that this is the wrong approach. Implementing WITH >> TIES and PERCENT together using an implicit window function call >> kills two birds with one very small stone (the only executor change >> needed would be teaching LIMIT to be able to

Re: FETCH FIRST clause WITH TIES option

On 10/29/2018 04:17 PM, Andrew Gierth wrote: "Tomas" == Tomas Vondra writes: > On 10/26/2018 12:28 PM, Surafel Temesgen wrote: >> hello , >> >> The WITH TIES keyword is sql standard that specifies any peers of >> retained rows to retained in the result set too .which means >>

Re: FETCH FIRST clause WITH TIES option

> "Tomas" == Tomas Vondra writes: > On 10/26/2018 12:28 PM, Surafel Temesgen wrote: >> hello , >> >> The WITH TIES keyword is sql standard that specifies any peers of >> retained rows to retained in the result set too .which means >> according to ordering key the result set can

Re: FETCH FIRST clause WITH TIES option

Hello Surafel, On 10/26/2018 12:28 PM, Surafel Temesgen wrote: > hello , > > The WITH TIES keyword is sql standard that specifies any peers of > retained rows to retained in the result set too .which means > according to ordering key the result set can includes additional rows > which have

FETCH FIRST clause WITH TIES option

hello , The WITH TIES keyword is sql standard that specifies any peers of retained rows to retained in the result set too .which means according to ordering key the result set can includes additional rows which have ties on the last position, if there are any and It work with ORDER BY query The