From: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: [HACKERS] Bug in ExecModifyTable function and trigger issues for foreign tables |
Date: | 2017-11-27 11:19:31 |
Message-ID: | CAEZATCUGUnUoDJC=bf-L_vdMjMKDxUXN+XudkuN5wf4FDKNT=A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 26 November 2017 at 22:56, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>> [ fix-rewrite-tlist-v4.patch ]
>
> I started reviewing this patch. I did not much like the fact that it
> effectively moved rewriteTargetListUD to a different file and renamed it.
> That seems like unnecessary code churn, plus it breaks the analogy with
> rewriteTargetListIU, plus it will make back-patching harder (since that
> code isn't exactly the same in back branches). I see little reason why
> we can't leave it where it is and just make it non-static. It's not like
> there's no other parts of the rewriter that the planner calls.
>
I wonder if, years from now, it might look a bit odd that
rewriteTargetListUD() is doing part of work of preptlist.c, is only
called from there, and yet is located in the rewriter.
Aside from having a similar name to rewriteTargetListIU(), what
rewriteTargetListUD() does seems more like what
preprocess_targetlist() does for rowmarks. The fact that
rewriteTargetListIU() intentionally only applies to the parent,
whereas preprocess_targetlist() and now rewriteTargetListUD() apply to
each child, further destroys any similarity between
rewriteTargetListUD() and rewriteTargetListIU().
The point about back-patching is a reasonable one though, so I won't
mind either way.
A separate point -- it might be marginally more efficient to have the
work of rewriteTargetListUD() done after expand_targetlist() to avoid
the possible renumbering of the resjunk entries.
Regards,
Dean
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2017-11-27 11:54:49 | Re: explain analyze output with parallel workers - question about meaning of information for explain.depesz.com |
Previous Message | Thomas Munro | 2017-11-27 10:47:05 | Re: ERROR: too many dynamic shared memory segments |