From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: some more error location support |
Date: | 2018-08-29 13:11:50 |
Message-ID: | a3f11e02-0032-bd81-752f-1f2a96f2102d@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 27/08/2018 11:17, Fabien COELHO wrote:
> About patch 3: applies cleanly independently of the 2 others, compiles,
> "make check" is okay.
>
> A few comments:
>
> There seems to be several somehow unrelated changes: one about copy,
> one about trigger and one about constraints? The two later changes do not
> seem to impact the tests, though.
added more tests
> In "CreateTrigger", you moved "make_parsestate" but removed
> "free_parsestate". I'd rather move it than remove it.
See also previous discussion, but I've moved it around for now.
> In "value.h", the added location field deserves a "/* token location, or
> -1 if unknown */" comment like others in "parsenode.h", "plannode.h" and
> "primnodes.h".
done
> Copying and comparing values are updaed, but value in/out functions are
> not updated to read/write the location, although other objects have their
> location serialized. ISTM that it should be done as well.
Hmm, maybe that's a problem, because the serialization of a Value node
is just a scalar. It doesn't have any structure where to put additional
fields. Maybe we should think about not using Value as a parse
representation for column name lists. Let me think about that.
Attached is another patch set. I think the first two patches are OK
now, but the third one might need to be rethought.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Error-position-support-for-defaults-and-check-con.patch | text/plain | 4.3 KB |
v2-0002-Error-position-support-for-partition-specificatio.patch | text/plain | 5.0 KB |
v2-0003-Add-location-information-to-Value-nodes.patch | text/plain | 16.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2018-08-29 13:24:38 | Re: Removing useless \. at the end of copy in pgbench |
Previous Message | Peter Eisentraut | 2018-08-29 13:08:46 | Re: some more error location support |