Hi, On Fri, Feb 28, 2020 at 4:06 PM legrand legrand <legrand_legr...@hotmail.com> wrote: > > It seems IVM team does not consider this point as a priority ...
Well, IVM is a big project and I agree that fixing this issue isn't the most urgent one, especially since there's no guarantee that this pgss planning patch will be committed, or with the current behavior. > We should not wait for them, if we want to keep a chance to be > included in PG13. > > So we have to make this feature more robust, an assert failure being > considered as a severe regression (even if this is not coming from pgss). I'm still not convinced that handling NULL query string, as in sometimes ignoring planning counters, is the right solution here. For now all code is able to provide it (or at least all the code that goes through make installcheck). I'm wondering if it'd be better to instead add a similar assert in pg_plan_query, to make sure that this requirements is always met even without using pg_stat_statements, or any other extension that would also rely on that. I also realized that the last version of the patch I sent was a rebase of the wrong version, I'll send the correct version soon. > I like the idea of adding a check for a non-zero queryId in the new > pgss_planner_hook() (zero queryid shouldn't be reserved for > utility_statements ?). Some assert hit later, I can say that it's not always true. For instance a CREATE TABLE AS won't run parse analysis for the underlying query, as this has already been done for the original statement, but will still call the planner. I'll change pgss_planner_hook to ignore such cases, as pgss_store would otherwise think that it's a utility statement. That'll probably incidentally fix the IVM incompatibility.
