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 majordomo@postgresql.org)

Neil Conway <neilc@samurai.com> 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

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

Neil Conway <neilc@samurai.com> 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 majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

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

Neil Conway <neilc@samurai.com> 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