On 28 December 2016 at 08:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I would not say that the current patch is giant & invasive, if you
>> abstract the two added fields to high-level statements.
>
> It's touching every single utility statement type, which is not only
> pretty invasive in itself, but will break any pending patches that
> add more utility statement types. And you've not followed through on the
> implications of adding those fields in, for instance, src/backend/nodes/;
> so the patch will be even bigger once all the required tidying-up is done.
I echo Tom's complaint. Adding fields to every node is definitely not
the way to do this.
>> I understand that you suggest to add a new intermediate structure:
>
>> typedef struct {
>> NodeTag tag;
>> int location, length;
>> Node *stmt;
>> } ParseNode;
>
>> So that raw_parser would return a List<ParseNode> instead of a List<Node>,
>> and the statements would be unmodified.
>
> Yeah, that's what I was thinking of.
I strongly agree. That's what I'd done in my draft patch, but I hadn't
got the details of actually collecting position tracking info sorted
out yet.
The information will need to be copied to plans later, but that's
easy, and would've been necessary anyway.
-- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Remote DBA, Training
&Services