public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v20 1/2] Expand EPQ tests for UPDATEs and DELETEs
19+ messages / 9 participants
[nested] [flat]
* [PATCH v20 1/2] Expand EPQ tests for UPDATEs and DELETEs
@ 2019-03-20 18:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Andres Freund @ 2019-03-20 18:44 UTC (permalink / raw)
Previously there was basically no coverage for UPDATEs encountering
deleted rows, and no coverage for DELETE having to perform EPQ. That's
problematic for an upcoming commit in which EPQ is tought to integrate
with tableams.
Author: Andres Freund
---
.../isolation/expected/eval-plan-qual.out | 218 +++++++++++++++++-
src/test/isolation/specs/eval-plan-qual.spec | 33 ++-
2 files changed, 241 insertions(+), 10 deletions(-)
diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out
index bbbb62ef4b1..bab01e0788a 100644
--- a/src/test/isolation/expected/eval-plan-qual.out
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -1,10 +1,16 @@
Parsed test spec with 3 sessions
starting permutation: wx1 wx2 c1 c2 read
-step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking';
-step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking'; <waiting ...>
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+400
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; <waiting ...>
step c1: COMMIT;
step wx2: <... completed>
+balance
+
+850
step c2: COMMIT;
step read: SELECT * FROM accounts ORDER BY accountid;
accountid balance
@@ -13,10 +19,15 @@ checking 850
savings 600
starting permutation: wy1 wy2 c1 c2 read
-step wy1: UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking';
-step wy2: UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000; <waiting ...>
+step wy1: UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1100
+step wy2: UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000 RETURNING balance; <waiting ...>
step c1: COMMIT;
step wy2: <... completed>
+balance
+
step c2: COMMIT;
step read: SELECT * FROM accounts ORDER BY accountid;
accountid balance
@@ -24,6 +35,195 @@ accountid balance
checking 1100
savings 600
+starting permutation: wx1 wx2 r1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+400
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; <waiting ...>
+step r1: ROLLBACK;
+step wx2: <... completed>
+balance
+
+1050
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+checking 1050
+savings 600
+
+starting permutation: wy1 wy2 r1 c2 read
+step wy1: UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1100
+step wy2: UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000 RETURNING balance; <waiting ...>
+step r1: ROLLBACK;
+step wy2: <... completed>
+balance
+
+1600
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+checking 1600
+savings 600
+
+starting permutation: wx1 d1 wx2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+400
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance;
+balance
+
+400
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; <waiting ...>
+step c1: COMMIT;
+step wx2: <... completed>
+balance
+
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+savings 600
+
+starting permutation: wx2 d1 c2 c1 read
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance; <waiting ...>
+step c2: COMMIT;
+step d1: <... completed>
+balance
+
+1050
+step c1: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+savings 600
+
+starting permutation: wx2 wx2 d1 c2 c1 read
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1500
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance; <waiting ...>
+step c2: COMMIT;
+step d1: <... completed>
+balance
+
+step c1: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+checking 1500
+savings 600
+
+starting permutation: wx2 d2 d1 c2 c1 read
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
+step d2: DELETE FROM accounts WHERE accountid = 'checking';
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance; <waiting ...>
+step c2: COMMIT;
+step d1: <... completed>
+balance
+
+step c1: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+savings 600
+
+starting permutation: wx1 d1 wx2 r1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+400
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance;
+balance
+
+400
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; <waiting ...>
+step r1: ROLLBACK;
+step wx2: <... completed>
+balance
+
+1050
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+checking 1050
+savings 600
+
+starting permutation: wx2 d1 r2 c1 read
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance; <waiting ...>
+step r2: ROLLBACK;
+step d1: <... completed>
+balance
+
+600
+step c1: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+savings 600
+
+starting permutation: wx2 wx2 d1 r2 c1 read
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1500
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance; <waiting ...>
+step r2: ROLLBACK;
+step d1: <... completed>
+balance
+
+600
+step c1: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+savings 600
+
+starting permutation: wx2 d2 d1 r2 c1 read
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
+step d2: DELETE FROM accounts WHERE accountid = 'checking';
+step d1: DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance; <waiting ...>
+step r2: ROLLBACK;
+step d1: <... completed>
+balance
+
+600
+step c1: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid balance
+
+savings 600
+
starting permutation: upsert1 upsert2 c1 c2 read
step upsert1:
WITH upsert AS
@@ -106,7 +306,10 @@ a b c
step c2: COMMIT;
starting permutation: wx2 partiallock c2 c1 read
-step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking';
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
step partiallock:
SELECT * FROM accounts a1, accounts a2
WHERE a1.accountid = a2.accountid
@@ -126,7 +329,10 @@ checking 1050
savings 600
starting permutation: wx2 lockwithvalues c2 c1 read
-step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking';
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
+balance
+
+1050
step lockwithvalues:
SELECT * FROM accounts a1, (values('checking'),('savings')) v(id)
WHERE a1.accountid = v.id
diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec
index 2e1b5095e8e..c22bf65f42f 100644
--- a/src/test/isolation/specs/eval-plan-qual.spec
+++ b/src/test/isolation/specs/eval-plan-qual.spec
@@ -42,9 +42,16 @@ teardown
session "s1"
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
# wx1 then wx2 checks the basic case of re-fetching up-to-date values
-step "wx1" { UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking'; }
+step "wx1" { UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance; }
# wy1 then wy2 checks the case where quals pass then fail
-step "wy1" { UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking'; }
+step "wy1" { UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking' RETURNING balance; }
+
+# d1 then wx1 checks that update can deal with the updated row vanishing
+# wx2 then d1 checks that the delete affects the updated row
+# wx2, wx2 then d1 checks that the delete checks the quals correctly (balance too high)
+# wx2, d2, then d1 checks that delete handles a vanishing row correctly
+step "d1" { DELETE FROM accounts WHERE accountid = 'checking' AND balance < 1500 RETURNING balance; }
+
# upsert tests are to check writable-CTE cases
step "upsert1" {
WITH upsert AS
@@ -64,6 +71,7 @@ step "readp1" { SELECT tableoid::regclass, ctid, * FROM p WHERE b IN (0, 1) AND
step "writep1" { UPDATE p SET b = -1 WHERE a = 1 AND b = 1 AND c = 0; }
step "writep2" { UPDATE p SET b = -b WHERE a = 1 AND c = 0; }
step "c1" { COMMIT; }
+step "r1" { ROLLBACK; }
# these tests are meant to exercise EvalPlanQualFetchRowMarks,
# ie, handling non-locked tables in an EvalPlanQual recheck
@@ -128,8 +136,10 @@ step "selectresultforupdate" {
session "s2"
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
-step "wx2" { UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking'; }
-step "wy2" { UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000; }
+step "wx2" { UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; }
+step "wy2" { UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000 RETURNING balance; }
+step "d2" { DELETE FROM accounts WHERE accountid = 'checking'; }
+
step "upsert2" {
WITH upsert AS
(UPDATE accounts SET balance = balance + 1234
@@ -161,6 +171,7 @@ step "updateforcip3" {
step "wrtwcte" { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; }
step "wrjt" { UPDATE jointest SET data = 42 WHERE id = 7; }
step "c2" { COMMIT; }
+step "r2" { ROLLBACK; }
session "s3"
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
@@ -192,8 +203,22 @@ step "multireadwcte" {
teardown { COMMIT; }
+# test that normal update follows update chains, and reverifies quals
permutation "wx1" "wx2" "c1" "c2" "read"
permutation "wy1" "wy2" "c1" "c2" "read"
+permutation "wx1" "wx2" "r1" "c2" "read"
+permutation "wy1" "wy2" "r1" "c2" "read"
+
+# test that deletes follow chains, and reverifies quals
+permutation "wx1" "d1" "wx2" "c1" "c2" "read"
+permutation "wx2" "d1" "c2" "c1" "read"
+permutation "wx2" "wx2" "d1" "c2" "c1" "read"
+permutation "wx2" "d2" "d1" "c2" "c1" "read"
+permutation "wx1" "d1" "wx2" "r1" "c2" "read"
+permutation "wx2" "d1" "r2" "c1" "read"
+permutation "wx2" "wx2" "d1" "r2" "c1" "read"
+permutation "wx2" "d2" "d1" "r2" "c1" "read"
+
permutation "upsert1" "upsert2" "c1" "c2" "read"
permutation "readp1" "writep1" "readp2" "c1" "c2"
permutation "writep2" "returningp1" "c1" "c2"
--
2.21.0.dirty
--qw3ipai4e5xhdytn
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v20-0002-tableam-Add-insert-delete-update-lock_tuple.patch"
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
@ 2024-08-15 07:59 Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-09-12 02:07 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest David Rowley <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Jelte Fennema-Nio @ 2024-08-15 07:59 UTC (permalink / raw)
To: Euler Taveira <[email protected]>; +Cc: Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 15 Aug 2024 at 05:17, Euler Taveira <[email protected]> wrote:
> +1. Regarding the CI link, I would be good if the CF entry automatically adds a
> link to the CI run. It can be a separate field or even add it to "Links".
I'm on it. I think this email should be a subset of the info on the CF
entry webpage, so I'll first change the cf entry page to include all
this info.
> I'm not sure about 4, you can always check the latest patch in the CF entry (it
> is usually the "latest attachment") and that's what the cfbot uses to run.
This is definitely a personal preference thing, but I like reading
patches on GitHub much better than looking at raw patch files. It has
syntax highlighting and has those little arrow buttons at the top of a
diff, to show more context about the file.
I realized a 5th thing that I would want in the email and cf entry page
5. A copy-pastable set of git command that checks out the patch by
downloading it from the cfbot repo like this:
git config branch.cf/5107.remote
https://github.com/postgresql-cfbot/postgresql.git
git config branch.cf/5107.merge refs/heads/cf/5107
git checkout -b cf/5107
git pull
> If I understand your proposal correctly, there will be another email to the
> thread if the previous CF was closed and someone opened a new CF entry.
> Sometimes some CF entries are about the same thread.
Yeah, that's correct. If a new CF entry is created for an existing
thread a new email would be sent. But to be clear, if CF entry is
pushed to the next commitfest, **no** new email would be sent.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
@ 2024-08-15 13:33 ` Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-16 18:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Geoghegan <[email protected]>
1 sibling, 2 replies; 19+ messages in thread
From: Peter Eisentraut @ 2024-08-15 13:33 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; Euler Taveira <[email protected]>; +Cc: Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On 15.08.24 09:59, Jelte Fennema-Nio wrote:
> I realized a 5th thing that I would want in the email and cf entry page
>
> 5. A copy-pastable set of git command that checks out the patch by
> downloading it from the cfbot repo like this:
>
> git config branch.cf/5107.remote
> https://github.com/postgresql-cfbot/postgresql.git
> git config branch.cf/5107.merge refs/heads/cf/5107
> git checkout -b cf/5107
> git pull
Maybe this kind of thing should rather be on the linked-to web page, not
in every email.
But a more serious concern here is that the patches created by the cfbot
are not canonical. There are various heuristics when they get applied.
I would prefer that people work with the actual patches sent by email,
at least unless they know exactly what they are doing. We don't want to
create parallel worlds of patches that are like 90% similar but not
really identical.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
@ 2024-08-15 14:01 ` Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Jelte Fennema-Nio @ 2024-08-15 14:01 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Euler Taveira <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 15 Aug 2024 at 15:33, Peter Eisentraut <[email protected]> wrote:
> Maybe this kind of thing should rather be on the linked-to web page, not
> in every email.
Yeah, I'll first put a code snippet on the page for the commitfest entry.
> But a more serious concern here is that the patches created by the cfbot
> are not canonical. There are various heuristics when they get applied.
> I would prefer that people work with the actual patches sent by email,
> at least unless they know exactly what they are doing. We don't want to
> create parallel worlds of patches that are like 90% similar but not
> really identical.
I'm not really sure what kind of heuristics and resulting differences
you're worried about here. The heuristics it uses are very simple and
are good enough for our CI. Basically they are:
1. Unzip/untar based on file extension
2. Apply patches using "patch" in alphabetic order
Also, when I apply patches myself, I use heuristics too. And my
heuristics are probably different from yours. So I'd expect that many
people using the exact same heuristic would only make the situation
better. Especially because if people don't know exactly what they are
doing, then their heuristics are probably not as good as the one of
our cfbot. I know I've struggled a lot the first few times when I was
manually applying patches.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
@ 2024-08-15 17:25 ` Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Matthias van de Meent @ 2024-08-15 17:25 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Euler Taveira <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
(sorry for the formatting, my mobile phone doesn't have the capabilities I
usually get when using my laptop)
On Thu, 15 Aug 2024, 16:02 Jelte Fennema-Nio, <[email protected]> wrote:
> On Thu, 15 Aug 2024 at 15:33, Peter Eisentraut <[email protected]>
> wrote:
> > Maybe this kind of thing should rather be on the linked-to web page, not
> > in every email.
>
> Yeah, I'll first put a code snippet on the page for the commitfest entry.
>
> > But a more serious concern here is that the patches created by the cfbot
> > are not canonical. There are various heuristics when they get applied.
> > I would prefer that people work with the actual patches sent by email,
> > at least unless they know exactly what they are doing. We don't want to
> > create parallel worlds of patches that are like 90% similar but not
> > really identical.
>
> I'm not really sure what kind of heuristics and resulting differences
> you're worried about here. The heuristics it uses are very simple and
> are good enough for our CI. Basically they are:
> 1. Unzip/untar based on file extension
> 2. Apply patches using "patch" in alphabetic order
>
> Also, when I apply patches myself, I use heuristics too. And my
> heuristics are probably different from yours. So I'd expect that many
> people using the exact same heuristic would only make the situation
> better. Especially because if people don't know exactly what they are
> doing, then their heuristics are probably not as good as the one of
> our cfbot. I know I've struggled a lot the first few times when I was
> manually applying patches.
One serious issue with this is that in cases of apply failures, CFBot
delays, or other issues, the CFBot repo won't contain the latest version of
the series' patchsets. E.g. a hacker can accidentally send an incremental
patch, or an unrelated patch to fix an issue mentioned in the thread
without splitting into a different thread, etc. This can easily cause users
(and CFBot) to test and review the wrong patch, esp. when the mail thread
proper is not looked by the reviewer, which would be somewhat promoted by a
CFA+github -centric workflow.
Apart from the above issue, I'm -0.5 on what to me equates with automated
spam to -hackers: the volume of mails would put this around the 16th most
common sender on -hackers, with about 400 mails/year (based on 80 new
patches for next CF, and 5 CFs/year, combined with Robert's 2023 statistics
at [0]).
I also don't quite like the suggested contents of such mail: (1) and (2)
are essentially duplicative information, and because CF's entries' IDs are
not shown in the app the "with ID 0000" part of (1) is practically useless
(better use the CFE's title), (3) would best be stored and/or integrated in
the CFA, as would (4). Additionally, (4) isn't canonical/guaranteed to be
up-to-date, see above. As for the "copy-pastable git commands" suggestion,
I'm not sure that's applicable, for the same reasons that (4) won't work
reliably. CFBot's repo to me seems more like an internal implementation
detail of CFBot than an authorative source of patchset diffs.
Maybe we could instead automate CF mail thread registration by allowing
registration of threadless CF entries (as 'preliminary'), and detecting
(and subsequently linking) new threads containing references to those CF
entries, with e.g. an "CF: https://commitfest.postgresql.org/49/4980/";
directive in the new thread's initial mail's text. This would give the
benefits of requiring no second mail for CF referencing purposes, be it
automated or manual.
Alternatively, we could allow threads for new entries to be started through
the CF app (which would automatically insert the right form data into the
mail), providing an alternative avenue to registering patches that doesn't
have the chicken-and-egg problem you're trying to address here.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
[0] https://rhaas.blogspot.com/2024/01/who-contributed-to-postgresql.html
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
@ 2024-08-16 07:12 ` Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-08-19 13:10 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Peter Eisentraut @ 2024-08-16 07:12 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; Jelte Fennema-Nio <[email protected]>; +Cc: Euler Taveira <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On 15.08.24 19:25, Matthias van de Meent wrote:
> Apart from the above issue, I'm -0.5 on what to me equates with
> automated spam to -hackers: the volume of mails would put this around
> the 16th most common sender on -hackers, with about 400 mails/year
> (based on 80 new patches for next CF, and 5 CFs/year, combined with
> Robert's 2023 statistics at [0]).
Yeah, I'd rather not open the can of worms that we send automated emails
to this list at all. If we do this, then there will be other requests,
and why this one and not that one. If people want to get emails from
the commitfest app, it should be that you subscribe there and it sends
those emails to those who want them.
> I also don't quite like the suggested contents of such mail: (1) and (2)
> are essentially duplicative information, and because CF's entries' IDs
> are not shown in the app the "with ID 0000" part of (1) is practically
> useless (better use the CFE's title), (3) would best be stored and/or
> integrated in the CFA, as would (4). Additionally, (4) isn't
> canonical/guaranteed to be up-to-date, see above. As for the
> "copy-pastable git commands" suggestion, I'm not sure that's applicable,
> for the same reasons that (4) won't work reliably. CFBot's repo to me
> seems more like an internal implementation detail of CFBot than an
> authorative source of patchset diffs.
I agree. And this also smells a bit like "my favorite workflow". Maybe
start with a blog post or a wiki page if you want to suggest this.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
@ 2024-08-16 14:43 ` Tom Lane <[email protected]>
2024-08-19 13:31 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-25 06:54 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
1 sibling, 2 replies; 19+ messages in thread
From: Tom Lane @ 2024-08-16 14:43 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Matthias van de Meent <[email protected]>; Jelte Fennema-Nio <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
Peter Eisentraut <[email protected]> writes:
> On 15.08.24 19:25, Matthias van de Meent wrote:
>> Apart from the above issue, I'm -0.5 on what to me equates with
>> automated spam to -hackers: the volume of mails would put this around
>> the 16th most common sender on -hackers, with about 400 mails/year
>> (based on 80 new patches for next CF, and 5 CFs/year, combined with
>> Robert's 2023 statistics at [0]).
> Yeah, I'd rather not open the can of worms that we send automated emails
> to this list at all. If we do this, then there will be other requests,
> and why this one and not that one. If people want to get emails from
> the commitfest app, it should be that you subscribe there and it sends
> those emails to those who want them.
That would destroy the one good argument for this, which was to
provide an easy way to get from a mail list thread (in the archives)
to the corresponding CF entry or entries. You pull up the "flat"
thread, search for the bot mail, and click the link. Without that
I see little point at all.
However, there are other ways to accomplish that. I liked the
suggestion of extending the CF webapp with a way to search for entries
mentioning a particular mail message ID. I dunno how hard it'd be to
get it to recognize *any* message-ID from a thread, but surely at
least the head message ought not be too hard to match.
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
@ 2024-08-19 13:31 ` Jelte Fennema-Nio <[email protected]>
2024-08-19 14:16 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Jelte Fennema-Nio @ 2024-08-19 13:31 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Matthias van de Meent <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, 16 Aug 2024 at 16:43, Tom Lane <[email protected]> wrote:
> However, there are other ways to accomplish that. I liked the
> suggestion of extending the CF webapp with a way to search for entries
> mentioning a particular mail message ID. I dunno how hard it'd be to
> get it to recognize *any* message-ID from a thread, but surely at
> least the head message ought not be too hard to match.
I sent a patch to support this on the commitfest app to Magnus
off-list. It was pretty easy to implement, even for *any* message-ID.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-08-19 13:31 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
@ 2024-08-19 14:16 ` Tom Lane <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Tom Lane @ 2024-08-19 14:16 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Matthias van de Meent <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
Jelte Fennema-Nio <[email protected]> writes:
> On Fri, 16 Aug 2024 at 16:43, Tom Lane <[email protected]> wrote:
>> However, there are other ways to accomplish that. I liked the
>> suggestion of extending the CF webapp with a way to search for entries
>> mentioning a particular mail message ID. I dunno how hard it'd be to
>> get it to recognize *any* message-ID from a thread, but surely at
>> least the head message ought not be too hard to match.
> I sent a patch to support this on the commitfest app to Magnus
> off-list. It was pretty easy to implement, even for *any* message-ID.
Cool, thank you!
regards, tom lane
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
@ 2024-09-25 06:54 ` Jelte Fennema-Nio <[email protected]>
2024-09-25 07:10 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-26 15:05 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
1 sibling, 2 replies; 19+ messages in thread
From: Jelte Fennema-Nio @ 2024-09-25 06:54 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Matthias van de Meent <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, 16 Aug 2024 at 07:43, Tom Lane <[email protected]> wrote:
> However, there are other ways to accomplish that. I liked the
> suggestion of extending the CF webapp with a way to search for entries
> mentioning a particular mail message ID. I dunno how hard it'd be to
> get it to recognize *any* message-ID from a thread, but surely at
> least the head message ought not be too hard to match.
This is now deployed, so you can now find a CF entry based on the email ID.
A bunch of other improvements also got deployed:
- Improved homepage[1] (now with useful and bookmarkable links at the top)
- More links on the cf entry page[2] (cfbot results, github diff, and
stable link to entry itself)
- Instructions on how to checkout an cfbot entry
CFBot traffic lights directly on the cfentry and probably the
commifest list page are the next thing I'm planning to work on
After that I'll take a look at sending opt-in emails
Another thing that I'm interested in adding is some metric of patch
size, so it's easier to find small patches that are thus hopefully
"easy" to review. To accommodate multi-patch emails, I'm thinking of
showing lines changed in the first patch and lines changed in all
patches together. Possibly showing it clearly, if significantly more
lines were deleted than added, so it's easy to spot effective
refactorings.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-09-25 06:54 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
@ 2024-09-25 07:10 ` Jelte Fennema-Nio <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Jelte Fennema-Nio @ 2024-09-25 07:10 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Matthias van de Meent <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, 24 Sept 2024 at 23:54, Jelte Fennema-Nio <[email protected]> wrote:
> A bunch of other improvements also got deployed:
> - Improved homepage[1] (now with useful and bookmarkable links at the top)
> - More links on the cf entry page[2] (cfbot results, github diff, and
[1]: https://commitfest.postgresql.org/
[2]: https://commitfest.postgresql.org/patch/5070
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-09-25 06:54 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
@ 2024-09-26 15:05 ` Robert Haas <[email protected]>
2024-09-26 18:57 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Robert Haas @ 2024-09-26 15:05 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Matthias van de Meent <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, Sep 25, 2024 at 2:55 AM Jelte Fennema-Nio <[email protected]> wrote:
> Another thing that I'm interested in adding is some metric of patch
> size, so it's easier to find small patches that are thus hopefully
> "easy" to review. To accommodate multi-patch emails, I'm thinking of
> showing lines changed in the first patch and lines changed in all
> patches together. Possibly showing it clearly, if significantly more
> lines were deleted than added, so it's easy to spot effective
> refactorings.
I like this general idea. Anything that helps us figure out what to
pay attention to in the CommitFest is great stuff. Focusing on the
first patch seems odd to me, though: often the earlier patches will be
preparatory patches, so small, and the big patch is someplace near the
end of the series.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-09-25 06:54 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-26 15:05 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
@ 2024-09-26 18:57 ` Jelte Fennema-Nio <[email protected]>
2024-09-27 12:08 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Jelte Fennema-Nio @ 2024-09-26 18:57 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Matthias van de Meent <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 26 Sept 2024 at 08:06, Robert Haas <[email protected]> wrote:
> Focusing on the first patch seems odd to me, though
Indeed the first few patches will often be small, and the big patch
will appear later. When I split patches up, those small patches should
usually be reviewable without looking at the big patch in detail, and
hopefully they shouldn't be too contentious: e.g. a comment
improvement or some small refactor. But often those patches don't seem
to be reviewed significantly quicker or merged significantly earlier
than the big patch. That makes it seem to me that even though they
should be relatively low-risk to commit and low-effort to review,
reviewers are scared away by the sheer number of patches in the
patchset, or by the size of the final patch. That's why I thought it
could be useful to specifically show the size of the first patch in
addition to the total patchset size, so that reviewers can easily spot
some small hopefully easy to review patch at the start of a patchset.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-09-25 06:54 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-26 15:05 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
2024-09-26 18:57 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
@ 2024-09-27 12:08 ` Robert Haas <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Robert Haas @ 2024-09-27 12:08 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Matthias van de Meent <[email protected]>; Euler Taveira <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Sep 26, 2024 at 2:57 PM Jelte Fennema-Nio <[email protected]> wrote:
> On Thu, 26 Sept 2024 at 08:06, Robert Haas <[email protected]> wrote:
> > Focusing on the first patch seems odd to me, though
>
> Indeed the first few patches will often be small, and the big patch
> will appear later. When I split patches up, those small patches should
> usually be reviewable without looking at the big patch in detail, and
> hopefully they shouldn't be too contentious: e.g. a comment
> improvement or some small refactor. But often those patches don't seem
> to be reviewed significantly quicker or merged significantly earlier
> than the big patch. That makes it seem to me that even though they
> should be relatively low-risk to commit and low-effort to review,
> reviewers are scared away by the sheer number of patches in the
> patchset, or by the size of the final patch. That's why I thought it
> could be useful to specifically show the size of the first patch in
> addition to the total patchset size, so that reviewers can easily spot
> some small hopefully easy to review patch at the start of a patchset.
Fair enough! Personally what I'd want to know is how large the biggest
patch is, but I see your point, too.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
@ 2024-08-19 13:10 ` Robert Haas <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Robert Haas @ 2024-08-19 13:10 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Matthias van de Meent <[email protected]>; Jelte Fennema-Nio <[email protected]>; Euler Taveira <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Aug 16, 2024 at 3:13 AM Peter Eisentraut <[email protected]> wrote:
> Yeah, I'd rather not open the can of worms that we send automated emails
> to this list at all.
+1.
> If people want to get emails from
> the commitfest app, it should be that you subscribe there and it sends
> those emails to those who want them.
+1.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
@ 2024-08-16 18:12 ` Peter Geoghegan <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Peter Geoghegan @ 2024-08-16 18:12 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; Euler Taveira <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, Aug 15, 2024 at 9:33 AM Peter Eisentraut <[email protected]> wrote:
> But a more serious concern here is that the patches created by the cfbot
> are not canonical. There are various heuristics when they get applied.
It's true that the code required for CFBot to simply apply a patch is
nontrivial. We're accounting for various edge-cases, and soldiering
on, where we deem that it makes sense. I'm referring to those cases
where "git am" won't work perfectly, but we have a pretty good chance
of successfully generating what the patch author intended (I assume
that that's what you meant by "heuristics").
One reason why this works better than might be expected is
because...we then test the result (that's the whole point, of course).
Obviously, if we apply a patch in a way that isn't quite perfectly
clean (according to whatever the "git am" criteria is), but that CFBot
is nevertheless capable of applying, and we then find that the end
result passes all tests, that gives us a fairly strong signal. We can
have high confidence that CFBot has done the right thing at that
point. We can reasonably present the resulting feature branch as a
"known good" usable version of the patch.
Of course you can quibble with this. Fundamentally, a patch file can
never be updated, but we want to apply it on top of a moving target
(as best we can). We're always making some kind of trade-off. I just
don't think that the heuristics that humans might choose to apply are
necessarily much better on average. Humans are bad at routine boring
things, but good at noticing and coping with special cases.
> I would prefer that people work with the actual patches sent by email,
> at least unless they know exactly what they are doing. We don't want to
> create parallel worlds of patches that are like 90% similar but not
> really identical.
There's a reason why tech companies place so much emphasis on offering
a "low friction experience". The aggregate effect on user/consumer
behavior is huge. I'm not saying that this is good or bad (let's not
get into that now); just that it is an empirical fact that people tend
to behave like that. We want more reviewers. Why not try to meet
people where they are a bit more?
I have to admit that I think that I'd be far more likely to quickly
test out a patch if I'm presented with a workflow that makes the setup
as painless as possible. Particularly if I'm all but guaranteed to get
a working Postgres server with the patch applied (which is possible
when I'm building exactly the same feature branch as the one that
passed CI by CFBot). I'm entirely willing to believe that it wouldn't
work that way for you, but I'm confident that a lot of people are in
the same camp as me.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
@ 2024-09-12 02:07 ` David Rowley <[email protected]>
2024-09-12 08:44 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: David Rowley @ 2024-09-12 02:07 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Euler Taveira <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 15 Aug 2024 at 20:00, Jelte Fennema-Nio <[email protected]> wrote:
>
> On Thu, 15 Aug 2024 at 05:17, Euler Taveira <[email protected]> wrote:
> > If I understand your proposal correctly, there will be another email to the
> > thread if the previous CF was closed and someone opened a new CF entry.
> > Sometimes some CF entries are about the same thread.
>
> Yeah, that's correct. If a new CF entry is created for an existing
> thread a new email would be sent. But to be clear, if CF entry is
> pushed to the next commitfest, **no** new email would be sent.
I was thinking about this today when looking at [1], you can see at
the end of that email someone posted a link to the CF entry.
Unfortunately, that was for the Jan-2024 CF, which is not really that
useful to look at today.
It looks like only about 35% of patches in this CF have "num CFs" = 1,
so it might be annoying to be taken to an old entry 65% of the time
you click the proposed URL. Maybe instead of the URLs showing the CF
number that the patch was first added to, if it could have a reference
in the URL that looks up the maximum CF which contains the patch and
show that one. e.g. https://commitfest.postgresql.org/latest/4751/ .
Without that, if you try to modify the status of a patch in an older
CF, you'll just be told that you can't do that. (it is true you can
click the latest CF in the status column, but that's at least one more
click than I'd have hoped)
David
[1] https://postgr.es/m/CACJufxEdavJhkUDhJ1jraXnZ9ayNQU+TvjuQjzQbuGS06oNZEQ@mail.gmail.com
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-12 02:07 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest David Rowley <[email protected]>
@ 2024-09-12 08:44 ` Jelte Fennema-Nio <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Jelte Fennema-Nio @ 2024-09-12 08:44 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Euler Taveira <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>
On Thu, 12 Sept 2024 at 04:07, David Rowley <[email protected]> wrote:
> Maybe instead of the URLs showing the CF
> number that the patch was first added to, if it could have a reference
> in the URL that looks up the maximum CF which contains the patch and
> show that one. e.g. https://commitfest.postgresql.org/latest/4751/ .
Yeah agreed, that's why I added support for
https://commitfest.postgresql.org/patch/4751/ a while ago. That URL
will be easily discoverable on the commitfest entry page soon (patch
for this is in flight).
^ permalink raw reply [nested|flat] 19+ messages in thread
* [PATCH v23 1/8] Row pattern recognition patch for raw parser.
@ 2024-10-25 03:56 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Tatsuo Ishii @ 2024-10-25 03:56 UTC (permalink / raw)
---
src/backend/parser/gram.y | 221 ++++++++++++++++++++++++++++++--
src/include/nodes/parsenodes.h | 67 ++++++++++
src/include/parser/kwlist.h | 8 ++
src/include/parser/parse_node.h | 1 +
4 files changed, 286 insertions(+), 11 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index baca4059d2..ddfee6a652 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -664,6 +664,21 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
json_object_constructor_null_clause_opt
json_array_constructor_null_clause_opt
+%type <target> row_pattern_measure_item
+ row_pattern_definition
+%type <node> opt_row_pattern_common_syntax
+ opt_row_pattern_skip_to
+ row_pattern_subset_item
+ row_pattern_term
+%type <list> opt_row_pattern_measures
+ row_pattern_measure_list
+ row_pattern_definition_list
+ opt_row_pattern_subset_clause
+ row_pattern_subset_list
+ row_pattern_subset_rhs
+ row_pattern
+%type <boolean> opt_row_pattern_initial_or_seek
+ first_or_last
/*
* Non-keyword token types. These are hard-wired into the "flex" lexer.
@@ -707,7 +722,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
CURRENT_TIME CURRENT_TIMESTAMP CURRENT_USER CURSOR CYCLE
DATA_P DATABASE DAY_P DEALLOCATE DEC DECIMAL_P DECLARE DEFAULT DEFAULTS
- DEFERRABLE DEFERRED DEFINER DELETE_P DELIMITER DELIMITERS DEPENDS DEPTH DESC
+ DEFERRABLE DEFERRED DEFINE DEFINER DELETE_P DELIMITER DELIMITERS DEPENDS DEPTH DESC
DETACH DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P
DOUBLE_P DROP
@@ -723,7 +738,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
HANDLER HAVING HEADER_P HOLD HOUR_P
IDENTITY_P IF_P ILIKE IMMEDIATE IMMUTABLE IMPLICIT_P IMPORT_P IN_P INCLUDE
- INCLUDING INCREMENT INDENT INDEX INDEXES INHERIT INHERITS INITIALLY INLINE_P
+ INCLUDING INCREMENT INDENT INDEX INDEXES INHERIT INHERITS INITIAL INITIALLY INLINE_P
INNER_P INOUT INPUT_P INSENSITIVE INSERT INSTEAD INT_P INTEGER
INTERSECT INTERVAL INTO INVOKER IS ISNULL ISOLATION
@@ -736,7 +751,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P LOCKED LOGGED
- MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MERGE MERGE_ACTION METHOD
+ MAPPING MATCH MATCHED MATERIALIZED MAXVALUE MEASURES MERGE MERGE_ACTION METHOD
MINUTE_P MINVALUE MODE MONTH_P MOVE
NAME_P NAMES NATIONAL NATURAL NCHAR NESTED NEW NEXT NFC NFD NFKC NFKD NO
@@ -748,8 +763,9 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
ORDER ORDINALITY OTHERS OUT_P OUTER_P
OVER OVERLAPS OVERLAY OVERRIDING OWNED OWNER
- PARALLEL PARAMETER PARSER PARTIAL PARTITION PASSING PASSWORD PATH
- PERIOD PLACING PLAN PLANS POLICY
+ PARALLEL PARAMETER PARSER PARTIAL PARTITION PASSING PASSWORD PAST
+ PATH PATTERN_P PERIOD PERMUTE PLACING PLAN PLANS POLICY
+
POSITION PRECEDING PRECISION PRESERVE PREPARE PREPARED PRIMARY
PRIOR PRIVILEGES PROCEDURAL PROCEDURE PROCEDURES PROGRAM PUBLICATION
@@ -760,12 +776,13 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
RESET RESTART RESTRICT RETURN RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP
ROUTINE ROUTINES ROW ROWS RULE
- SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT
+ SAVEPOINT SCALAR SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SEEK SELECT
SEQUENCE SEQUENCES
+
SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW
SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SOURCE SQL_P STABLE STANDALONE_P
START STATEMENT STATISTICS STDIN STDOUT STORAGE STORED STRICT_P STRING_P STRIP_P
- SUBSCRIPTION SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER
+ SUBSCRIPTION SUBSET SUBSTRING SUPPORT SYMMETRIC SYSID SYSTEM_P SYSTEM_USER
TABLE TABLES TABLESAMPLE TABLESPACE TARGET TEMP TEMPLATE TEMPORARY TEXT_P THEN
TIES TIME TIMESTAMP TO TRAILING TRANSACTION TRANSFORM
@@ -874,6 +891,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%nonassoc UNBOUNDED NESTED /* ideally would have same precedence as IDENT */
%nonassoc IDENT PARTITION RANGE ROWS GROUPS PRECEDING FOLLOWING CUBE ROLLUP
SET KEYS OBJECT_P SCALAR VALUE_P WITH WITHOUT PATH
+ MEASURES AFTER INITIAL SEEK PATTERN_P
%left Op OPERATOR /* multi-character ops and user-defined operators */
%left '+' '-'
%left '*' '/' '%'
@@ -16279,7 +16297,8 @@ over_clause: OVER window_specification
;
window_specification: '(' opt_existing_window_name opt_partition_clause
- opt_sort_clause opt_frame_clause ')'
+ opt_sort_clause opt_row_pattern_measures opt_frame_clause
+ opt_row_pattern_common_syntax ')'
{
WindowDef *n = makeNode(WindowDef);
@@ -16287,10 +16306,12 @@ window_specification: '(' opt_existing_window_name opt_partition_clause
n->refname = $2;
n->partitionClause = $3;
n->orderClause = $4;
+ n->rowPatternMeasures = $5;
/* copy relevant fields of opt_frame_clause */
- n->frameOptions = $5->frameOptions;
- n->startOffset = $5->startOffset;
- n->endOffset = $5->endOffset;
+ n->frameOptions = $6->frameOptions;
+ n->startOffset = $6->startOffset;
+ n->endOffset = $6->endOffset;
+ n->rpCommonSyntax = (RPCommonSyntax *)$7;
n->location = @1;
$$ = n;
}
@@ -16314,6 +16335,31 @@ opt_partition_clause: PARTITION BY expr_list { $$ = $3; }
| /*EMPTY*/ { $$ = NIL; }
;
+/*
+ * ROW PATTERN_P MEASURES
+ */
+opt_row_pattern_measures: MEASURES row_pattern_measure_list { $$ = $2; }
+ | /*EMPTY*/ { $$ = NIL; }
+ ;
+
+row_pattern_measure_list:
+ row_pattern_measure_item
+ { $$ = list_make1($1); }
+ | row_pattern_measure_list ',' row_pattern_measure_item
+ { $$ = lappend($1, $3); }
+ ;
+
+row_pattern_measure_item:
+ a_expr AS ColLabel
+ {
+ $$ = makeNode(ResTarget);
+ $$->name = $3;
+ $$->indirection = NIL;
+ $$->val = (Node *) $1;
+ $$->location = @1;
+ }
+ ;
+
/*
* For frame clauses, we return a WindowDef, but only some fields are used:
* frameOptions, startOffset, and endOffset.
@@ -16473,6 +16519,143 @@ opt_window_exclusion_clause:
| /*EMPTY*/ { $$ = 0; }
;
+opt_row_pattern_common_syntax:
+opt_row_pattern_skip_to opt_row_pattern_initial_or_seek
+ PATTERN_P '(' row_pattern ')'
+ opt_row_pattern_subset_clause
+ DEFINE row_pattern_definition_list
+ {
+ RPCommonSyntax *n = makeNode(RPCommonSyntax);
+ n->rpSkipTo = ((RPCommonSyntax *)$1)->rpSkipTo;
+ n->rpSkipVariable = ((RPCommonSyntax *)$1)->rpSkipVariable;
+ n->initial = $2;
+ n->rpPatterns = $5;
+ n->rpSubsetClause = $7;
+ n->rpDefs = $9;
+ $$ = (Node *) n;
+ }
+ | /*EMPTY*/ { $$ = NULL; }
+ ;
+
+opt_row_pattern_skip_to:
+ AFTER MATCH SKIP TO NEXT ROW
+ {
+ RPCommonSyntax *n = makeNode(RPCommonSyntax);
+ n->rpSkipTo = ST_NEXT_ROW;
+ n->rpSkipVariable = NULL;
+ $$ = (Node *) n;
+ }
+ | AFTER MATCH SKIP PAST LAST_P ROW
+ {
+ RPCommonSyntax *n = makeNode(RPCommonSyntax);
+ n->rpSkipTo = ST_PAST_LAST_ROW;
+ n->rpSkipVariable = NULL;
+ $$ = (Node *) n;
+ }
+ | AFTER MATCH SKIP TO first_or_last ColId
+ {
+ RPCommonSyntax *n = makeNode(RPCommonSyntax);
+ n->rpSkipTo = $5? ST_FIRST_VARIABLE : ST_LAST_VARIABLE;
+ n->rpSkipVariable = $6;
+ $$ = (Node *) n;
+ }
+/*
+ | AFTER MATCH SKIP TO LAST_P ColId %prec LAST_P
+ {
+ RPCommonSyntax *n = makeNode(RPCommonSyntax);
+ n->rpSkipTo = ST_LAST_VARIABLE;
+ n->rpSkipVariable = $6;
+ $$ = n;
+ }
+ | AFTER MATCH SKIP TO ColId
+ {
+ RPCommonSyntax *n = makeNode(RPCommonSyntax);
+ n->rpSkipTo = ST_VARIABLE;
+ n->rpSkipVariable = $5;
+ $$ = n;
+ }
+*/
+ | /*EMPTY*/
+ {
+ RPCommonSyntax *n = makeNode(RPCommonSyntax);
+ /* temporary set default to ST_NEXT_ROW */
+ n->rpSkipTo = ST_PAST_LAST_ROW;
+ n->rpSkipVariable = NULL;
+ $$ = (Node *) n;
+ }
+ ;
+
+first_or_last:
+ FIRST_P { $$ = true; }
+ | LAST_P { $$ = false; }
+ ;
+
+opt_row_pattern_initial_or_seek:
+ INITIAL { $$ = true; }
+ | SEEK
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("SEEK is not supported"),
+ errhint("Use INITIAL."),
+ parser_errposition(@1)));
+ }
+ | /*EMPTY*/ { $$ = true; }
+ ;
+
+row_pattern:
+ row_pattern_term { $$ = list_make1($1); }
+ | row_pattern row_pattern_term { $$ = lappend($1, $2); }
+ ;
+
+row_pattern_term:
+ ColId { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "", (Node *)makeString($1), NULL, @1); }
+ | ColId '*' { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "*", (Node *)makeString($1), NULL, @1); }
+ | ColId '+' { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "+", (Node *)makeString($1), NULL, @1); }
+ | ColId '?' { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "?", (Node *)makeString($1), NULL, @1); }
+ ;
+
+opt_row_pattern_subset_clause:
+ SUBSET row_pattern_subset_list { $$ = $2; }
+ | /*EMPTY*/ { $$ = NIL; }
+ ;
+
+row_pattern_subset_list:
+ row_pattern_subset_item { $$ = list_make1($1); }
+ | row_pattern_subset_list ',' row_pattern_subset_item { $$ = lappend($1, $3); }
+ | /*EMPTY*/ { $$ = NIL; }
+ ;
+
+row_pattern_subset_item: ColId '=' '(' row_pattern_subset_rhs ')'
+ {
+ RPSubsetItem *n = makeNode(RPSubsetItem);
+ n->name = $1;
+ n->rhsVariable = $4;
+ $$ = (Node *) n;
+ }
+ ;
+
+row_pattern_subset_rhs:
+ ColId { $$ = list_make1(makeStringConst($1, @1)); }
+ | row_pattern_subset_rhs ',' ColId { $$ = lappend($1, makeStringConst($3, @1)); }
+ | /*EMPTY*/ { $$ = NIL; }
+ ;
+
+row_pattern_definition_list:
+ row_pattern_definition { $$ = list_make1($1); }
+ | row_pattern_definition_list ',' row_pattern_definition { $$ = lappend($1, $3); }
+ ;
+
+row_pattern_definition:
+ ColId AS a_expr
+ {
+ $$ = makeNode(ResTarget);
+ $$->name = $1;
+ $$->indirection = NIL;
+ $$->val = (Node *) $3;
+ $$->location = @1;
+ }
+ ;
/*
* Supporting nonterminals for expressions.
@@ -17683,6 +17866,7 @@ unreserved_keyword:
| INDEXES
| INHERIT
| INHERITS
+ | INITIAL
| INLINE_P
| INPUT_P
| INSENSITIVE
@@ -17711,6 +17895,7 @@ unreserved_keyword:
| MATCHED
| MATERIALIZED
| MAXVALUE
+ | MEASURES
| MERGE
| METHOD
| MINUTE_P
@@ -17755,8 +17940,11 @@ unreserved_keyword:
| PARTITION
| PASSING
| PASSWORD
+ | PAST
| PATH
+ | PATTERN_P
| PERIOD
+ | PERMUTE
| PLAN
| PLANS
| POLICY
@@ -17808,6 +17996,7 @@ unreserved_keyword:
| SEARCH
| SECOND_P
| SECURITY
+ | SEEK
| SEQUENCE
| SEQUENCES
| SERIALIZABLE
@@ -17835,6 +18024,7 @@ unreserved_keyword:
| STRING_P
| STRIP_P
| SUBSCRIPTION
+ | SUBSET
| SUPPORT
| SYSID
| SYSTEM_P
@@ -18029,6 +18219,7 @@ reserved_keyword:
| CURRENT_USER
| DEFAULT
| DEFERRABLE
+ | DEFINE
| DESC
| DISTINCT
| DO
@@ -18192,6 +18383,7 @@ bare_label_keyword:
| DEFAULTS
| DEFERRABLE
| DEFERRED
+ | DEFINE
| DEFINER
| DELETE_P
| DELIMITER
@@ -18269,6 +18461,7 @@ bare_label_keyword:
| INDEXES
| INHERIT
| INHERITS
+ | INITIAL
| INITIALLY
| INLINE_P
| INNER_P
@@ -18323,6 +18516,7 @@ bare_label_keyword:
| MATCHED
| MATERIALIZED
| MAXVALUE
+ | MEASURES
| MERGE
| MERGE_ACTION
| METHOD
@@ -18379,8 +18573,11 @@ bare_label_keyword:
| PARTITION
| PASSING
| PASSWORD
+ | PAST
| PATH
+ | PATTERN_P
| PERIOD
+ | PERMUTE
| PLACING
| PLAN
| PLANS
@@ -18438,6 +18635,7 @@ bare_label_keyword:
| SCROLL
| SEARCH
| SECURITY
+ | SEEK
| SELECT
| SEQUENCE
| SEQUENCES
@@ -18471,6 +18669,7 @@ bare_label_keyword:
| STRING_P
| STRIP_P
| SUBSCRIPTION
+ | SUBSET
| SUBSTRING
| SUPPORT
| SYMMETRIC
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b40b661ec8..685ca438b1 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -552,6 +552,48 @@ typedef struct SortBy
ParseLoc location; /* operator location, or -1 if none/unknown */
} SortBy;
+/*
+ * AFTER MATCH row pattern skip to types in row pattern common syntax
+ */
+typedef enum RPSkipTo
+{
+ ST_NONE, /* AFTER MATCH omitted */
+ ST_NEXT_ROW, /* SKIP TO NEXT ROW */
+ ST_PAST_LAST_ROW, /* SKIP TO PAST LAST ROW */
+ ST_FIRST_VARIABLE, /* SKIP TO FIRST variable name */
+ ST_LAST_VARIABLE, /* SKIP TO LAST variable name */
+ ST_VARIABLE /* SKIP TO variable name */
+} RPSkipTo;
+
+/*
+ * Row Pattern SUBSET clause item
+ */
+typedef struct RPSubsetItem
+{
+ NodeTag type;
+ char *name; /* Row Pattern SUBSET clause variable name */
+ List *rhsVariable; /* Row Pattern SUBSET rhs variables (list of
+ * char *string) */
+} RPSubsetItem;
+
+/*
+ * RowPatternCommonSyntax - raw representation of row pattern common syntax
+ *
+ */
+typedef struct RPCommonSyntax
+{
+ NodeTag type;
+ RPSkipTo rpSkipTo; /* Row Pattern AFTER MATCH SKIP type */
+ char *rpSkipVariable; /* Row Pattern Skip To variable name, if any */
+ bool initial; /* true if <row pattern initial or seek> is
+ * initial */
+ List *rpPatterns; /* PATTERN variables (list of A_Expr) */
+ List *rpSubsetClause; /* row pattern subset clause (list of
+ * RPSubsetItem), if any */
+ List *rpDefs; /* row pattern definitions clause (list of
+ * ResTarget) */
+} RPCommonSyntax;
+
/*
* WindowDef - raw representation of WINDOW and OVER clauses
*
@@ -567,6 +609,9 @@ typedef struct WindowDef
char *refname; /* referenced window name, if any */
List *partitionClause; /* PARTITION BY expression list */
List *orderClause; /* ORDER BY (list of SortBy) */
+ List *rowPatternMeasures; /* row pattern measures (list of
+ * ResTarget) */
+ RPCommonSyntax *rpCommonSyntax; /* row pattern common syntax */
int frameOptions; /* frame_clause options, see below */
Node *startOffset; /* expression for starting bound, if any */
Node *endOffset; /* expression for ending bound, if any */
@@ -1530,6 +1575,11 @@ typedef struct GroupingSet
* the orderClause might or might not be copied (see copiedOrder); the framing
* options are never copied, per spec.
*
+ * "defineClause" is Row Pattern Recognition DEFINE clause (list of
+ * TargetEntry). TargetEntry.resname represents row pattern definition
+ * variable name. "patternVariable" and "patternRegexp" represents PATTERN
+ * clause.
+ *
* The information relevant for the query jumbling is the partition clause
* type and its bounds.
*/
@@ -1559,6 +1609,23 @@ typedef struct WindowClause
Index winref; /* ID referenced by window functions */
/* did we copy orderClause from refname? */
bool copiedOrder pg_node_attr(query_jumble_ignore);
+ /* Row Pattern AFTER MACH SKIP clause */
+ RPSkipTo rpSkipTo; /* Row Pattern Skip To type */
+ char *rpSkipVariable; /* Row Pattern Skip To variable */
+ bool initial; /* true if <row pattern initial or seek> is
+ * initial */
+ /* Row Pattern DEFINE clause (list of TargetEntry) */
+ List *defineClause;
+ /* Row Pattern DEFINE variable initial names (list of String) */
+ List *defineInitial;
+ /* Row Pattern PATTERN variable name (list of String) */
+ List *patternVariable;
+
+ /*
+ * Row Pattern PATTERN regular expression quantifier ('+' or ''. list of
+ * String)
+ */
+ List *patternRegexp;
} WindowClause;
/*
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 899d64ad55..f52d797615 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -129,6 +129,7 @@ PG_KEYWORD("default", DEFAULT, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("defaults", DEFAULTS, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("deferrable", DEFERRABLE, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("deferred", DEFERRED, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("define", DEFINE, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("definer", DEFINER, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("delete", DELETE_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("delimiter", DELIMITER, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -215,6 +216,7 @@ PG_KEYWORD("index", INDEX, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("indexes", INDEXES, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("inherit", INHERIT, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("inherits", INHERITS, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("initial", INITIAL, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("initially", INITIALLY, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("inline", INLINE_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("inner", INNER_P, TYPE_FUNC_NAME_KEYWORD, BARE_LABEL)
@@ -273,6 +275,7 @@ PG_KEYWORD("match", MATCH, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("matched", MATCHED, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("materialized", MATERIALIZED, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("maxvalue", MAXVALUE, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("measures", MEASURES, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("merge", MERGE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("merge_action", MERGE_ACTION, COL_NAME_KEYWORD, BARE_LABEL)
PG_KEYWORD("method", METHOD, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -337,8 +340,11 @@ PG_KEYWORD("partial", PARTIAL, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("partition", PARTITION, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("passing", PASSING, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("password", PASSWORD, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("past", PAST, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("path", PATH, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("pattern", PATTERN_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("period", PERIOD, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("permute", PERMUTE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("placing", PLACING, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("plan", PLAN, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("plans", PLANS, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -399,6 +405,7 @@ PG_KEYWORD("scroll", SCROLL, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("search", SEARCH, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("second", SECOND_P, UNRESERVED_KEYWORD, AS_LABEL)
PG_KEYWORD("security", SECURITY, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("seek", SEEK, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("select", SELECT, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("sequence", SEQUENCE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("sequences", SEQUENCES, UNRESERVED_KEYWORD, BARE_LABEL)
@@ -432,6 +439,7 @@ PG_KEYWORD("strict", STRICT_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("string", STRING_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("strip", STRIP_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("subscription", SUBSCRIPTION, UNRESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("subset", SUBSET, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("substring", SUBSTRING, COL_NAME_KEYWORD, BARE_LABEL)
PG_KEYWORD("support", SUPPORT, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("symmetric", SYMMETRIC, RESERVED_KEYWORD, BARE_LABEL)
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 2375e95c10..737f8a9e0e 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -51,6 +51,7 @@ typedef enum ParseExprKind
EXPR_KIND_WINDOW_FRAME_RANGE, /* window frame clause with RANGE */
EXPR_KIND_WINDOW_FRAME_ROWS, /* window frame clause with ROWS */
EXPR_KIND_WINDOW_FRAME_GROUPS, /* window frame clause with GROUPS */
+ EXPR_KIND_RPR_DEFINE, /* DEFINE */
EXPR_KIND_SELECT_TARGET, /* SELECT target list item */
EXPR_KIND_INSERT_TARGET, /* INSERT target list item */
EXPR_KIND_UPDATE_SOURCE, /* UPDATE assignment source item */
--
2.25.1
----Next_Part(Fri_Oct_25_13_04_53_2024_648)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v23-0002-Row-pattern-recognition-patch-parse-analysis.patch"
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2024-10-25 03:56 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 18:44 [PATCH v20 1/2] Expand EPQ tests for UPDATEs and DELETEs Andres Freund <[email protected]>
2024-08-15 07:59 Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 13:33 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-15 14:01 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-15 17:25 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Matthias van de Meent <[email protected]>
2024-08-16 07:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Eisentraut <[email protected]>
2024-08-16 14:43 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-08-19 13:31 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-08-19 14:16 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Tom Lane <[email protected]>
2024-09-25 06:54 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-25 07:10 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-26 15:05 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
2024-09-26 18:57 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-09-27 12:08 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
2024-08-19 13:10 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Robert Haas <[email protected]>
2024-08-16 18:12 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Peter Geoghegan <[email protected]>
2024-09-12 02:07 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest David Rowley <[email protected]>
2024-09-12 08:44 ` Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest Jelte Fennema-Nio <[email protected]>
2024-10-25 03:56 [PATCH v23 1/8] Row pattern recognition patch for raw parser. Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox