Re: [HACKERS] read-only planner input
Tom Lane wrote: I'd go with PlannerState. QueryState for some reason sounds more like execution-time state. Well, not to me :) It just makes sense to me that QueryState as the working state associated with a Query. Not sure it makes a big difference, though. Pulling the planner internal stuff out of the Query node does seem like a good idea, even so. I've got most of this finished; I'll post a patch soon. One issue I ran into is how to handle query_tree_mutator() and query_tree_walker(): they both expect to be able to traverse a Query's in_info_list, which my patch moves into the QueryState struct. If maintaining this property is essential, it seems that we'll need a way to get the QueryState associated with a given Query. We can't just change the query tree walker to be a query state walker, since we need to be able to recurse into subqueries, and the RTE for a subquery will only contain a Query, not its QueryState. Any thoughts on the best way to fix this? -Neil ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] read-only planner input
Neil Conway [EMAIL PROTECTED] writes: I've got most of this finished; I'll post a patch soon. One issue I ran into is how to handle query_tree_mutator() and query_tree_walker(): they both expect to be able to traverse a Query's in_info_list, which my patch moves into the QueryState struct. If maintaining this property is essential, it seems that we'll need a way to get the QueryState associated with a given Query. whereupon the entire premise of a read-only tree collapses ... That's a bit nasty. I'm fairly sure that I added in_info_list to the walker recursion because I had to; I don't recall the exact scenario, but I think it needs to be possible to reassign relation numbers within that data structure if we are doing it elsewhere in a query tree. regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] read-only planner input
Tom Lane wrote:
That's a bit nasty. I'm fairly sure that I added in_info_list to the
walker recursion because I had to; I don't recall the exact scenario,
but I think it needs to be possible to reassign relation numbers
within that data structure if we are doing it elsewhere in a query
tree.
It was r1.125 of clauses.c, and yes, it seems pretty important. It's
used in adjust_inherited_attrs_mutator() in prep/prepunion.c,
flatten_join_alias_vars_mutator() in util/var.c, and three different
walkers in rewriteManip.c. That's from trawling through the original
(Jan of '03) patch -- it may have been used elsewhere subsequently.
Here's one idea to fix this: when planning a Query, transform the Query
into a PlannedQuery. This would essentially be the same as the
QueryState we discussed earlier, except that we would also walk through
the Query and adjust references to nested Queries to refer to
PlannedQueries instead (so RTEs for subqueries would reference the
PlannedQuery, not the Query, for example). There would then be a
planned query walker that would walk both the original query and
additional planner-specific working state, and so on.
Perhaps we could use some trickery to avoid the PlannedQuery vs. Query
distinction when a particular piece of code doesn't care, by making
Query the first field of PlannedQuery. In other words:
struct PlannedQuery {
Query q;
/* other fields */
};
So we could treat a PlannedQuery * like a Query *. I don't really like
this solution.
Another possibility would be to punt, and keep in_info_list as part of
Query. We'd then need to resolve modifications to it in the same we way
will need to resolve modifications to legitimate parts of the Query
(e.g. by making an initial shallow copy and avoiding destructive
updates, per earlier discussion).
-Neil
---(end of broadcast)---
TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] read-only planner input
Neil Conway [EMAIL PROTECTED] writes:
Here's one idea to fix this: when planning a Query, transform the Query
into a PlannedQuery. This would essentially be the same as the
QueryState we discussed earlier, except that we would also walk through
the Query and adjust references to nested Queries to refer to
PlannedQueries instead (so RTEs for subqueries would reference the
PlannedQuery, not the Query, for example). There would then be a
planned query walker that would walk both the original query and
additional planner-specific working state, and so on.
Perhaps we could use some trickery to avoid the PlannedQuery vs. Query
distinction when a particular piece of code doesn't care, by making
Query the first field of PlannedQuery. In other words:
struct PlannedQuery {
Query q;
/* other fields */
};
So we could treat a PlannedQuery * like a Query *. I don't really like
this solution.
No. At that point you've essentially booted away the entire point of
the change :-(
IIRC one of the main reasons for wanting to make the planner read-only
is so that it does *not* modify subquery RTE contents --- there are all
sorts of uglinesses involved in the fact that it presently does, mainly
having to be sure that we plan each subquery exactly once. If we go
this route then we won't be able to fix any of that stuff.
Another possibility would be to punt, and keep in_info_list as part of
Query.
That's seeming like the path of least resistance at the moment ... but
it still isn't going to solve the subquery RTE issues. I'm feeling a
bit discouraged about this concept right now ... maybe we need to back
off and think about a fresh start.
regards, tom lane
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] read-only planner input
Tom Lane wrote: You could make a good case that we just ought to save query text and start from there in any replanning; it'd be the most compact representation, the easiest to copy around, and the least likely to break. What happens if (for example) DateStyle changes between the two parses? (not that I'm sure what the expected behaviour is in that case anyway..) -O ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] read-only planner input
Oliver Jowett wrote: What happens if (for example) DateStyle changes between the two parses? From my original email: This is the common case of a more general problem: a query plan depends on various parts of the environment at plan-creation time. That environment includes the definitions of database objects, but also GUC variables (most importantly search_path, but also optimizer-tuning variables for example), the state of database statistics, and so on. I'll leave resolution of the more general problem to someone else -- I think if we can manage to invalidate plans automatically when dependent objects change, that's better than nothing. I'm don't think recreating the plan from the query string changes this fundamentally -- the interaction between (for example) GUC variables and prepared plans will likely not be well-defined (which will be no worse than it is today). (It might be plausible to work something out where a backend-local SET of some subset of the GUC variables flushes all the cached query plans, but I'm don't have any plans to look at it myself.) -Neil ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] read-only planner input
Neil Conway [EMAIL PROTECTED] writes: Oliver Jowett wrote: What happens if (for example) DateStyle changes between the two parses? I'm don't think recreating the plan from the query string changes this fundamentally -- the interaction between (for example) GUC variables and prepared plans will likely not be well-defined (which will be no worse than it is today). It is well defined, because we insist that the gram.y transformation not depend on any changeable state. So if we forced replan after a change of DateStyle, the right thing would happen with either plain text or raw parsetrees as the starting point. There is a separate issue here of course, which is whether it's really the right thing --- if the plan got through parsing the first time then it's pretty likely that it would fail under a different datestyle. But certainly we've seen requests for cached plans to respond to changes in search_path, and I doubt you can make a principled distinction between that and datestyle. From a practical point of view, I suspect the path of least resistance is to use plain text as the saved representation, because (a) it's smaller and (b) IIRC we don't currently have a complete set of copyfuncs for raw parsetree nodes. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] read-only planner input
Tom Lane wrote: It is well defined, because we insist that the gram.y transformation not depend on any changeable state. That's my point -- whether we begin from the query string or the raw parsetree shouldn't make a difference. By not well-defined, I meant that if the user is changing GUC variables on the fly, they can't rely on their prepared query being planned under any particular datestyle (or search path, etc.), since they can't really predict when replanning will take place (e.g. an sinval overflow could occur spontaneously and cause all cached plans to be invalidated). This is similar to how search_path and pl/pgsql works right now -- we'll use the search_path in effect when the query is planned, which may or may not be what the user would expect. -Neil ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] read-only planner input
Neil Conway [EMAIL PROTECTED] writes: ... By not well-defined, I meant that if the user is changing GUC variables on the fly, they can't rely on their prepared query being planned under any particular datestyle (or search path, etc.), since they can't really predict when replanning will take place (e.g. an sinval overflow could occur spontaneously and cause all cached plans to be invalidated). This is similar to how search_path and pl/pgsql works right now -- we'll use the search_path in effect when the query is planned, which may or may not be what the user would expect. As soon as we have the replanning mechanism, I think that there will be considerable pressure to use it to ensure that search_path changes and so on *do* behave consistently. The question here is what does consistently mean. My first thought is that the best idea in most scenarios would be to associate a search_path value with each function, rather than allowing the meaning of queries within each function to vary depending on the caller's search_path. We have seen one or two examples where the user would rather have the other behavior --- but they are surely a small minority. I'm not sure though that I care to extend that thought to each individual GUC variable ... even though it's surely true that changes in e.g. datestyle could break a particular function. The contrary argument is that GUC variables are useful just because they represent a single place to set some behavior, and having to fix the values associated with N different functions would be a nightmare outweighing the benefits of stable values for individual functions. Any thoughts on how this stuff ought to behave? regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[HACKERS] read-only planner input
I've been taking a look at how to stop the planner from scribbling on its input. This is my first modification of any significance to the planner, so don't hesitate to tell me what I've gotten wrong :) I think the planner makes two kinds of modifications to the input Query: (a) rewriting of the Query to improve planning (b) as a convenient place to store planner working state. Some examples of the former include transforming IN clauses to joins, transforming simple FROM-clause subselects into joins, preprocessing expressions, and so forth. Examples of the latter are mostly the internal to planner fields denoted in the Query struct definition. (b) should be pretty easy to solve; we can create a per-Query PlanState struct that contains this information, as well as holding a pointer to the Query (and perhaps the in-construct Plan tree). I'm still trying to figure out how to handle (a). Perhaps we can create an additional plan node that always sits at the top of the plan tree. This would hold derivations of data from the input Query. A lot of the code that implements (a) is actually already applicative in nature, but any code that modifies a Query destructively would need to be changed. In other words, rather than query-jointree = pull_up_subqueries(parse, query-jointree); We'd have: top_plan_node-jointree = pull_up_subqueries(plan_state, query-jointree); (Possibly passing PlanState rather than `parse', which is a Query, if needed. The example is also somewhat simplified.) BTW, I wonder whether it would be possible to move some preprocessing from the early stages of the planner to a preprocessing phase that would run after the rewriter but before the planner proper. The preprocessor would maintain the essential properties of the input Query, but it wouldn't need to be re-run when the query is replanned due to a modification to a dependent database object. For example, the decision about whether to pull-up a subquery could be done once and not redone in subsequent invocations of the planner on the same Query. On the other hand, I'm not sure how much preprocessing could be rearranged like this, and since replanning ought to be relatively rare, I'm not sure it's worth spending a whole lot of time trying to optimize it... Comments welcome. -Neil ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] read-only planner input
Neil Conway wrote: (b) should be pretty easy to solve; we can create a per-Query PlanState struct that contains this information, as well as holding a pointer to the Query (and perhaps the in-construct Plan tree). I just noticed that there is a `PlanState' node in the executor, of all places. I'm thinking of using `QueryState' instead -- this parallels the usage of PlanState in the executor, to some degree (PlanState holds some of the state of the executor as it examines a Plan; QueryState holds some of the state of the planner as it examines a Query). I also considered `PlannerState', but that doesn't seem best, as there can be multiple instances of this struct in existence for a single invocation of planner() (due to subqueries). I also created a new header file, `plan_internals.h' for the declaration of this struct, and moved a few planner-internal declarations from planner.h to plan_internals.h -- I think it would be best to only define external interfaces in planner.h -Neil ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] read-only planner input
Neil Conway [EMAIL PROTECTED] I've been taking a look at how to stop the planner from scribbling on its input. This is my first modification of any significance to the planner, so don't hesitate to tell me what I've gotten wrong :) So is this change the preparation work of caching query plans? Like cleaning the plans so they could be well shared? Regards, Qingqing ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] read-only planner input
Qingqing Zhou wrote: So is this change the preparation work of caching query plans? Like cleaning the plans so they could be well shared? Yeah, it is somewhat related to the centralized plan caching module that Tom and I have been discussing in the cached plan invalidation thread. When a cached plan has become invalidated, we want to re-run the planner on the original Query that has been saved away. If the planner modifies its input, that means we need to copy the Query it before passing it to the planner. While that is trivial to do, it is enough of an ugly hack that I decided to sit down and fix this problem before doing the rest of the cached plan invalidation work. Hopefully I haven't bitten off more than I can chew, so to speak :) -Neil ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] read-only planner input
Neil Conway [EMAIL PROTECTED] writes: I just noticed that there is a `PlanState' node in the executor, of all places. I'm thinking of using `QueryState' instead -- this parallels the usage of PlanState in the executor, to some degree (PlanState holds some of the state of the executor as it examines a Plan; QueryState holds some of the state of the planner as it examines a Query). I also considered `PlannerState', but that doesn't seem best, as there can be multiple instances of this struct in existence for a single invocation of planner() (due to subqueries). I'd go with PlannerState. QueryState for some reason sounds more like execution-time state. I don't buy the notion that PlannerState sounds like something there should be only one of; or at least, by the time you've found out that the planner is potentially re-entrant, that should not bother you ... I'm not really sure what to do about the problem of plan-foo = process(plan-foo) ... it's just *such* a convenient notation. Maybe it would be sufficient to legislate that you can only do that when you know that you are working with a copied plan object. The various recursive process routines can be designed to return fresh structure whenever they change anything --- for the most part we do that already, I think. So this might just boil down to having to make a flat copy of the top-level Query node before we start whacking at it. Pulling the planner internal stuff out of the Query node does seem like a good idea, even so. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] read-only planner input
Neil Conway [EMAIL PROTECTED] writes: BTW, I wonder whether it would be possible to move some preprocessing from the early stages of the planner to a preprocessing phase that would run after the rewriter but before the planner proper. The preprocessor would maintain the essential properties of the input Query, but it wouldn't need to be re-run when the query is replanned due to a modification to a dependent database object. I don't believe there is any very significant amount of planner work that is completely independent of any external database object. For that matter, even the rewriter needs to be rerun when any views or defaults change in the query. And for that matter, even the parse analysis phase is dependent on external definitions. It's fairly likely that the plan cache cannot safely use any upstream representation later than the raw parse tree that's output by gram.y. You could make a good case that we just ought to save query text and start from there in any replanning; it'd be the most compact representation, the easiest to copy around, and the least likely to break. Which of course calls into question whether your current thoughts about making the planner read-only are really going to advance the plan caching project at all. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] read-only planner input
Tom Lane wrote: I don't believe there is any very significant amount of planner work that is completely independent of any external database object. For that matter, even the rewriter needs to be rerun when any views or defaults change in the query. And for that matter, even the parse analysis phase is dependent on external definitions. It's fairly likely that the plan cache cannot safely use any upstream representation later than the raw parse tree that's output by gram.y. Hmm, I suppose that's true -- I had in mind that we would track dependencies with sufficient granularity that we would know when invoking each of those modules is necessary. For example, when a rule is added to the database that affects one of the dependent tables of a plan, we needn't rerun the parser or the analyzer. But on reflection I think you're right -- the scheme above doesn't really buy us much, and it is much simpler to just start with the query string or raw parsetree whenever we need to recreate an invalidate plan. Plus the above scheme might lead to some subtle bugs. Which of course calls into question whether your current thoughts about making the planner read-only are really going to advance the plan caching project at all. True, but I've crossed the Rubicon already :) (Actually, I might stop after I've introduced the QueryState struct and moved planner-internal fields out of Query -- that will at least be a significant step closer to the goal.) -Neil ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
