Hello Craig,

[...] 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.

Yep, but it is limited to headers and the break is trivial...

I'm not sure how else that part can be done.

We can add a common parent for all utility statement types, but that also touches all utility statements, and since we're working with pure C, it means 'type' won't be accessible as e.g. someCreateStmt.type, but only as something like ((UtilityStatement)someCreateStmt).type or someCreateStmt.hdr.type. [...] I don't think that's so bad, but it's not exactly less invasive.

I thought of this way of implementing that on the submitted version, I decided against because then it is less obvious what is directly in the structure, existing code may reference the tag field, and the number of header changes is the same, if a little less verbose.

[...] For fixing the information in pg_stat_statement, the location data must be transported from the parsed node to the query to the planned node, because the later two nodes types are passed to different hooks.


Now the detail is that utility statements, which seems to be nearly all of
them but select/update/delete/insert, do not have plans: The statement
itself is its own plan... so there is no place to store the location &

Yeah. I don't see any way around adding location info for utility
statements one way or the other.

If utility statements are done this way, that's 95% of all statements, so the point of doing the remaining 5% with Tom's neat intermediate node trick seems void, I think that it is better to have all of them the same way.

TBH I think that for utility statements, adding a member struct with
location info is the least-bad option. Something like:

typedef struct StmtLocation
   int offset;
   int length;

It is possible, but:

typedef struct CreateStmt
       NodeTag         type;
       StmtLocation  location;

Name "location" is already used meaning the offset or the directory destination for create table space, that would create a third option.

For homogeneity, ISTM that keeping location & length directly and renaming the table space location is the simplest & most homogeneous option.


Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to