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]> 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 13:33 Peter Eisentraut <[email protected]> parent: Jelte Fennema-Nio <[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 14:01 Jelte Fennema-Nio <[email protected]> parent: Peter Eisentraut <[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 17:25 Matthias van de Meent <[email protected]> parent: Jelte Fennema-Nio <[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-16 07:12 Peter Eisentraut <[email protected]> parent: Matthias van de Meent <[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-16 14:43 Tom Lane <[email protected]> parent: Peter Eisentraut <[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-16 18:12 Peter Geoghegan <[email protected]> parent: Peter Eisentraut <[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-19 13:10 Robert Haas <[email protected]> parent: Peter Eisentraut <[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-19 13:31 Jelte Fennema-Nio <[email protected]> parent: 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-19 14:16 Tom Lane <[email protected]> parent: Jelte Fennema-Nio <[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-09-12 02:07 David Rowley <[email protected]> parent: 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-09-12 08:44 Jelte Fennema-Nio <[email protected]> parent: David Rowley <[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
* Re: Opinion poll: Sending an automated email to a thread when it gets added to the commitfest @ 2024-09-25 06:54 Jelte Fennema-Nio <[email protected]> parent: Tom Lane <[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-09-25 07:10 Jelte Fennema-Nio <[email protected]> parent: 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-09-26 15:05 Robert Haas <[email protected]> parent: 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-09-26 18:57 Jelte Fennema-Nio <[email protected]> parent: 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-09-27 12:08 Robert Haas <[email protected]> parent: Jelte Fennema-Nio <[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
* [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