On 28 December 2016 at 08:33, Tom Lane <t...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers