From: | Atri Sharma <atri(dot)jiit(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Marko Tiikkaja <marko(at)joh(dot)to>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Support UPDATE table SET(*)=... |
Date: | 2014-12-15 10:50:26 |
Message-ID: | CAOeZVid1iuz7K-hp3-Vr5Ec_cUHdY=fYzWNu+yqAMUTruFKQOw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Dec 8, 2014 at 6:06 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:
>
> On Wed, Nov 26, 2014 at 4:26 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > I think what's likely missing here is a clear design for the raw parse
> > tree representation (what's returned by the bison grammar). The patch
> > seems to be trying to skate by without creating any new parse node types
> > or fields, but that may well be a bad idea. At the very least there
> > needs to be some commentary added to parsenodes.h explaining what the
> > representation actually is (cf commentary there for MultiAssignRef).
> >
> > Also, I think it's a mistake not to be following the MultiAssignRef
> > model for the case of "(*) = ctext_row". We optimize the ROW-source
> > case at the grammar stage when there's a fixed number of target columns,
> > because that's a very easy transformation --- but you can't do it like
> > that when there's not. It's possible that that optimization should be
> > delayed till later even in the existing case; in general, optimizing
> > in gram.y is a bad habit that's better avoided ...
> Marking as "returned with feedback" based on those comments.
>
>
Thank you everybody for your comments.
I am indeed trying to avoid a new node since my intuitive feeling says that
we do not need a new node for a functionality which is present in other
commands albeit in different form. However, I agree that documentation
explaining parse representation is lacking and shall improve that. Also, in
accordance to Tom's comments, I shall look for not touching target list
directly and follow the path of MultiAssignRef.
I have moved patch to current CF and marked it as Waiting on Author since I
plan to resubmit after addressing the concerns.
Regards,
Atri
--
Regards,
Atri
*l'apprenant*
From | Date | Subject | |
---|---|---|---|
Next Message | José Luis Tallón | 2014-12-15 11:09:45 | Re: On partitioning |
Previous Message | Dilip kumar | 2014-12-15 10:48:24 | Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ] |