Re: support for MERGE
Tomas Vondra <tomas.vondra@enterprisedb.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Simon Riggs <simon.riggs@enterprisedb.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
Pg Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-01-13T12:43:31Z
Lists: pgsql-hackers
On 1/13/21 11:20 AM, Simon Riggs wrote: > On Sun, Jan 10, 2021 at 1:44 AM Tomas Vondra > <tomas.vondra@enterprisedb.com> wrote: > >> 5) WHEN AND >> >> I admit the "WHEN AND" conditions sounds a bit cryptic - it took me a >> while to realize what this refers to. Is that a term established by SQL >> Standard, or something we invented? > > As Vik notes, this refers to the WHEN [NOT] MATCHED AND when-and-clause > so in that case I was referring to the "when-and_clause" portion. > Yes, that is part of the standard. > Yes, I know what it was referring to, and I know that the feature is per SQL standard. My point is that the "WHEN AND" term may be somewhat unclear, especially when used in a error message (which typically has very little context). I don't think SQL standard uses "WHEN AND" at all, it simply talks about <search conditions> and that's it. >> 6) walsender.c >> >> Huh, why does this patch touch this at all? > > Nothing I added, IIRC, nor am I aware of why that would exist. > >> 7) rewriteHandler.c >> >> I see MERGE "doesn't support" rewrite rules in the sense that it simply >> ignores them. Shouldn't it error-out instead? Seems like a foot-gun to >> me, because people won't realize this limitation and may not notice >> their rules don't fire. > > Simply ignoring rules is consistent with COPY, that was the only > reason for that choice. It could certainly throw an error instead. > Makes sense. >> 8) varlena.c >> >> Again, why are these changes to length checks in a MERGE patch? > > Nothing I added, IIRC, nor am I aware of why that would exist. > >> 9) parsenodes.h >> >> Should we rename mergeTarget_relation to mergeTargetRelation? The >> current name seems like a mix between two naming schemes. > > +1 > > We've had code from 4-5 people in the patch now, so I will re-review > myself to see if I can shed light on anything. > OK, thanks. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Commits
-
Doc fixes for MERGE statement
- 5dcebd10baa8 16.0 landed
- 5bb88967ed5c 15.0 landed
-
MERGE docs adjustments
- d6d1fbf353d1 15.0 landed
- 3d895bc846f2 16.0 landed
-
Link to MVCC docs in MERGE docs
- ffffeebf2447 15.0 landed
-
Fix EXPLAIN MERGE output when no tuples are processed
- 12e423e21d8e 15.0 landed
-
Make EXPLAIN MERGE output format more compact
- 598ac10be1c2 15.0 landed
-
Add support for MERGE SQL command
- 7103ebb7aae8 15.0 landed
-
Split ExecUpdate and ExecDelete into reusable pieces
- 25e777cf8e54 15.0 landed
-
Add API of sorts for transition table handling in trigger.c
- 3a46a45f6f00 15.0 landed
-
Represent Lists as expansible arrays, not chains of cons-cells.
- 1cff1b95ab6d 13.0 cited