Craig Ringer <craig.rin...@2ndquadrant.com> writes: > Sounds like it'd be better as a separate change so as not to block this one.
Yeah, if we do that at all it should be a separate patch IMO. The concerns are largely different. > I really like what you have done Tom, though I'm about to travel so I > haven't read it in full detail. Like Fabien I would've been certain that > it'd be rejected if I tried it, but I sure am glad you did it. Does anyone want to do further review on this before I push it? One thing that I'm not quite satisfied with is the business with non-top-level RawStmt nodes in some utility statements. That's certainly a wart from gram.y's perspective, and it's mostly a wart from analyze.c's perspective as well --- the parse analyze routines mostly just throw away the non-top-level RawStmt. The original reason for doing it was that DoCopy needs to call pg_analyze_and_rewrite() on the sub-statement, and I wanted pg_analyze_and_rewrite's argument to be declared as RawStmt, so CopyStmt's sub-statement had to have a RawStmt. And it seemed like a good idea for the other utility statements to be consistent with that. However, when all the dust settled, DoCopy ended up needing to construct a RawStmt node anyway for the case where it's inventing a Query tree to support an RLS query. We could make it construct a RawStmt node in the plain copy-from-query case too, and then there needn't be one in the grammar output. So I'm now thinking that it might be better if the grammar produced RawStmt only at top level, and anybody who calls pg_analyze_and_rewrite on sub-sections of a utility statement has to cons up a RawStmt to put at the top of the sub-query. This complicates life a little for utility statements that call parse analysis during execution, but the principle that RawStmt appears only at top level seems attractively simple. We don't nest PlannedStmts either. One thing that connects into this is why do we have some utility statements that do parse analysis of sub-statements immediately during their own parse analysis, while others postpone that to execution? I've not researched it yet, but my vague memory is that EXPLAIN and friends used to all do it in the second way, and we realized that that was broken so we changed them to the first way. COPY has only recently grown the ability to have a sub-query, and I'm wondering if it's just a failure of institutional memory that led us to do it the second way. If we ended up requiring all these cases to work more like EXPLAIN, they'd not be needing to construct RawStmts at execution anyway. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers