On Fri, Feb 27, 2026 at 8:16 PM David G. Johnston
<[email protected]> wrote:
> I have a mind to walk through the readmes and sgmls but its going to be in 
> chunks.  Here's one for the readme for pg_plan_advice with a couple of 
> preliminary sgml changes.

While I'm grateful for the feedback, I feel like you tend to suggest a
lot of edits that seem like they're just substituting your
idiosyncratic preferences for mine e.g. writing "types of scan" vs.
"scan types," or writing "additional, separate join problems" vs.
"independent join problems" or "judiciously" vs. "conservatively". I
don't really consider these to be improvements, nor do I necessarily
think they're worse, but I just don't see the point in litigating this
kind of stuff. If I've written something that is legitimately unclear,
or factually incorrect, or there's a spelling or punctuation mistake,
I'm happy to correct that kind of stuff, but I don't really want to go
through and replace a bunch of words that I liked with a bunch of
synonyms that you picked.

Also, when you just provide a diff like this, it's not that clear to
me why you're suggesting particular changes, which makes it hard to
decide whether I agree with them. And in a lot of cases I don't.
Looking at some particular examples:

+isn't going to work any more. That's expected. It should be resilient to
+changes in the statistics, including any CREATE STATISTICS related changes.

This is broadly true but seems a bit obvious to mention in a README.
If we were going to mention it, I'd think it would go in the
user-facing documentation. But I don't quite see why we should mention
it at all. If plan advice couldn't override changes caused by
statistics, what would even be the point of it? Also, it's not
categorically true in all situations, because as discussed elsewhere,
we have limitations like lack of control over aggregation strategy.

 Tags such as NESTED_LOOP_PLAIN specify the method that should be used to
-perform a certain join. More specifically, NESTED_LOOP_PLAIN(x (y z)) says
+perform a certain join - with the target appearing directly on the inner side
+of the join list first. Thus, NESTED_LOOP_PLAIN(x (y z)) says
 that the plan should put the relation whose identifier is "x" on the inner
 side of a plain nested loop (one without materialization or memoization)
 and that it should also put a join between the relation whose identifier is

This seems like you're adding a second explanation of what the
paragraph already goes onto say, except that the existing explanation
is more precise and detailed.

-useless in practice. It gives the planner too much freedom to do things that
+problematic in practice. It gives the planner too much freedom to do
things that

I mean, I stand by the word I picked. I don't want to weaken it.

-This means that if advice can say that a certain optimization or technique
-should be used, it should also be able to say that the optimization or
-technique should not be used. We should never assume that the absence of an
-instruction to do a certain thing means that it should not be done; all
-instructions must be explicit.
+In other words, advice tags must define whether they encourage or discourage
+certain optimizations or techniques. (NO_GATHER is an example of the latter.
+There is no generic "NOT" syntax, e.g., NOT(HASH_JOIN(dim2 dim4).))

My text explains an important design principle that future hackers
must keep in mind when modifying this system to avoid breaking
everything. Your replacement text just describes how it works today.
Considering that this is a README for hackers, I think that's much
worse.

-  advice" mini-language. It is intended to allow stabilization of plan choices
+  advice" domain specific language (DSL). It is intended to allow
stabilization of plan choices

There's a debate to be had about whether it's better to say
mini-language or domain specific language here, but it's hard for me
to decide which is better if all you provide is a diff replacing A
with B. I definitely think it's worse to write (DSL) here. There is no
point in defining an acronym if we're never going to use it anywhere.

-- 
Robert Haas
EDB: http://www.enterprisedb.com


Reply via email to