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.
> 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.
> 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.
--
Simon Riggs http://www.EnterpriseDB.com/
