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.
Yep.
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 &
length.
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.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
