From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: support for MERGE |
Date: | 2021-01-13 12:43:31 |
Message-ID: | 1e394ecf-74f1-a843-2bb6-edc0839eacf6@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
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(dot)vondra(at)enterprisedb(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2021-01-13 12:56:47 | Re: multi-install PostgresNode |
Previous Message | Daniel Gustafsson | 2021-01-13 12:25:01 | Re: multi-install PostgresNode |