Missing grammar production for WITH TIES
The syntax for FETCH FIRST allows the <fetch first quantity> to be
absent (implying 1).
We implement this correctly for ONLY, but WITH TIES didn't get the memo.
Patch attached.
--
Vik Fearing
Attachments:
with_ties.difftext/x-patch; charset=UTF-8; name=with_ties.diffDownload
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3c78f2d1b5..a24b30f06f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11816,6 +11816,14 @@ limit_clause:
n->limitOption = LIMIT_OPTION_COUNT;
$$ = n;
}
+ | FETCH first_or_next row_or_rows WITH TIES
+ {
+ SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+ n->limitOffset = NULL;
+ n->limitCount = makeIntConst(1, -1);
+ n->limitOption = LIMIT_OPTION_WITH_TIES;
+ $$ = n;
+ }
;
offset_clause:
On 2020-May-18, Vik Fearing wrote:
The syntax for FETCH FIRST allows the <fetch first quantity> to be
absent (implying 1).We implement this correctly for ONLY, but WITH TIES didn't get the memo.
Oops, yes. I added a test. Will get this pushed immediately after I
see beta1 produced.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-WITH-TIES-number-of-rows-is-optional-and-default-to-.patchtext/x-diff; charset=us-asciiDownload
From bdd724370215a550586e49a2f8ce2f554bdf79f4 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 18 May 2020 11:57:05 -0400
Subject: [PATCH] WITH TIES: number of rows is optional and default to one
Author: Vik Fearing
---
src/backend/parser/gram.y | 8 ++++++++
src/test/regress/expected/limit.out | 17 +++++++++++++++++
src/test/regress/sql/limit.sql | 5 +++++
3 files changed, 30 insertions(+)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 3c78f2d1b5..a24b30f06f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11816,6 +11816,14 @@ limit_clause:
n->limitOption = LIMIT_OPTION_COUNT;
$$ = n;
}
+ | FETCH first_or_next row_or_rows WITH TIES
+ {
+ SelectLimit *n = (SelectLimit *) palloc(sizeof(SelectLimit));
+ n->limitOffset = NULL;
+ n->limitCount = makeIntConst(1, -1);
+ n->limitOption = LIMIT_OPTION_WITH_TIES;
+ $$ = n;
+ }
;
offset_clause:
diff --git a/src/test/regress/expected/limit.out b/src/test/regress/expected/limit.out
index a4e175855c..e6f6809fbe 100644
--- a/src/test/regress/expected/limit.out
+++ b/src/test/regress/expected/limit.out
@@ -576,6 +576,23 @@ SELECT thousand
0
(10 rows)
+SELECT thousand
+ FROM onek WHERE thousand < 5
+ ORDER BY thousand FETCH FIRST ROWS WITH TIES;
+ thousand
+----------
+ 0
+ 0
+ 0
+ 0
+ 0
+ 0
+ 0
+ 0
+ 0
+ 0
+(10 rows)
+
SELECT thousand
FROM onek WHERE thousand < 5
ORDER BY thousand FETCH FIRST 1 ROW WITH TIES;
diff --git a/src/test/regress/sql/limit.sql b/src/test/regress/sql/limit.sql
index afce5019b2..d2d4ef132d 100644
--- a/src/test/regress/sql/limit.sql
+++ b/src/test/regress/sql/limit.sql
@@ -161,6 +161,10 @@ SELECT thousand
FROM onek WHERE thousand < 5
ORDER BY thousand FETCH FIRST 2 ROW WITH TIES;
+SELECT thousand
+ FROM onek WHERE thousand < 5
+ ORDER BY thousand FETCH FIRST ROWS WITH TIES;
+
SELECT thousand
FROM onek WHERE thousand < 5
ORDER BY thousand FETCH FIRST 1 ROW WITH TIES;
@@ -168,6 +172,7 @@ SELECT thousand
SELECT thousand
FROM onek WHERE thousand < 5
ORDER BY thousand FETCH FIRST 2 ROW ONLY;
+
-- should fail
SELECT ''::text AS two, unique1, unique2, stringu1
FROM onek WHERE unique1 > 50
--
2.20.1
On 5/18/20 7:03 PM, Alvaro Herrera wrote:
On 2020-May-18, Vik Fearing wrote:
The syntax for FETCH FIRST allows the <fetch first quantity> to be
absent (implying 1).We implement this correctly for ONLY, but WITH TIES didn't get the memo.
Oops, yes. I added a test. Will get this pushed immediately after I
see beta1 produced.
Thanks!
--
Vik Fearing
On 2020-May-18, Alvaro Herrera wrote:
On 2020-May-18, Vik Fearing wrote:
The syntax for FETCH FIRST allows the <fetch first quantity> to be
absent (implying 1).We implement this correctly for ONLY, but WITH TIES didn't get the memo.
Oops, yes. I added a test. Will get this pushed immediately after I
see beta1 produced.
Done. Thanks!
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, May 18, 2020 at 07:30:32PM -0400, Alvaro Herrera wrote:
Done. Thanks!
This has been committed just after beta1 has been stamped. So it
means that it won't be included in it, right?
--
Michael
On 5/19/20 4:36 AM, Michael Paquier wrote:
On Mon, May 18, 2020 at 07:30:32PM -0400, Alvaro Herrera wrote:
Done. Thanks!
This has been committed just after beta1 has been stamped. So it
means that it won't be included in it, right?
Correct.
I don't know why there was a delay, but it also doesn't bother me.
--
Vik Fearing
On 2020-May-19, Vik Fearing wrote:
On 5/19/20 4:36 AM, Michael Paquier wrote:
This has been committed just after beta1 has been stamped. So it
means that it won't be included in it, right?Correct.
Right.
I don't know why there was a delay, but it also doesn't bother me.
I didn't want to risk breaking the buildfarm at the last minute. It'll
be there in beta2.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Michael Paquier <michael@paquier.xyz> writes:
On Mon, May 18, 2020 at 07:30:32PM -0400, Alvaro Herrera wrote:
Done. Thanks!
This has been committed just after beta1 has been stamped. So it
means that it won't be included in it, right?
Right.
regards, tom lane
On Tue, May 19, 2020 at 12:41:39AM -0400, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
This has been committed just after beta1 has been stamped. So it
means that it won't be included in it, right?Right.
Still, wouldn't it be better to wait until the version is tagged? My
understanding is that we had better not commit anything on a branch
planned for release between the moment the version is stamped and the
moment the tag is pushed so as we have a couple of days to address any
complaints from -packagers. Here, we are in a state where we have
between the stamp time and tag time an extra commit not related to a
packaging issue. So, if it happens that we have an issue from
-packagers to address, then we would have to include c301c2e in the
beta1. Looking at the patch committed, that's not much of an issue,
but I think that we had better avoid that.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Tue, May 19, 2020 at 12:41:39AM -0400, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
This has been committed just after beta1 has been stamped. So it
means that it won't be included in it, right?
Right.
Still, wouldn't it be better to wait until the version is tagged?
Yeah, that would have been better per project protocol: if a tarball
re-wrap becomes necessary then it would be messy not to include this
change along with fixing whatever urgent bug there might be.
However, I thought the case for delaying this fix till post-wrap was kind
of thin anyway, so if that does happen I won't be too fussed about it.
Otherwise I would've said something earlier on this thread.
regards, tom lane
On 2020-May-19, Tom Lane wrote:
Yeah, that would have been better per project protocol: if a tarball
re-wrap becomes necessary then it would be messy not to include this
change along with fixing whatever urgent bug there might be.However, I thought the case for delaying this fix till post-wrap was kind
of thin anyway, so if that does happen I won't be too fussed about it.
Otherwise I would've said something earlier on this thread.
In the end, it's a judgement call. In this case, my assessment was that
the risk was small enough that I could push after I saw the tarballs
announced. In other cases I've judged differently and waited for
longer. If the fix had been even simpler, I would have pushed it right
away, but my confidence with grammar changes is not as high as I would
like.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services