Hello Tom,
How? The issue is that stmtmulti silently skip some ';' when empty
statements are found, [...]
Maybe you should undo that.
I was trying to be minimally invasive in the parser, i.e. not to change
any rules.
I've generally found that trying to put optimizations into the grammar
is a bad idea; it's better to KISS in gram.y and apply optimizations
later on.
I can change rules, but I'm not sure it will be better. It would allow to
remove the last ';' location in the lexer which Kyotar does not like, but
it would add some new stretching to compute the length of statements and
remove the empty statements.
I would say that it would be different, but not necessarily better.
- Make all parse nodes have the following two members at the
beginning. This unifies all parse node from the view of setting
their location. Commenting about this would be better.
| NodeTag type;
| int location;
I do not like this. You are making what should be a small patch into
a giant, invasive one.
I would not say that the current patch is giant & invasive, if you
abstract the two added fields to high-level statements.
I'd think about adding a single parse node type that corresponds to
"stmt" in the grammar and really doesn't do anything but carry the
location info for the overall statement. That info could then be
transferred into the resulting Query node.
I'm not sure I understand your suggestion. Currently I have added the
location & length information to all high-level statements nodes, plus
query and planned. I think that planned is necessary for utility
statements.
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.
Problem solved with (I think) very little code touched.
Hmmm...
Then raw_parser() callers have to manage a List<ParseNode> instead of the
List<Node>, I'm not sure of the size of the propagation to manage the
added indirection level appropriately: raw_parser is called 4 times (in
parser, tcop, commands, plpgsql...). The first call in tcop is
copyObject(), equal(), check_log_statement(), errdetail_execute()...
Probably it can be done, but I'm moderately unthousiastic: it looks like
starting unwinding a ball of wool, and I'm not sure where it would stop,
as it means touching at least the 4 uses of raw_parser in 4 distinct part
of postgres, whereas the current approach did not change anything for
raw_parser callers, although at the price of modifying all statement
nodes.
Did I read you correctly?
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
