On Wed, Apr 9, 2025 at 2:21 PM David Rowley <dgrowleyml@gmail.com> wrote:
> On Wed, 9 Apr 2025 at 17:09, Amit Langote <amitlangote09@gmail.com> wrote:
> > Should the following paragraph in src/backend/optimizer/README be
> > updated to reflect the new reality after recent changes?
> >
> > An EquivalenceClass can contain "em_is_child" members, which are copies
> > of members that contain appendrel parent relation Vars, transposed to
> > contain the equivalent child-relation variables or expressions. These
> > members are not full-fledged members of the EquivalenceClass and do not
> > affect the class's overall properties at all. They are kept only to
> > simplify matching of child-relation expressions to EquivalenceClasses.
> > Most operations on EquivalenceClasses should ignore child members.
> >
> > The part about these being in the EquivalenceClass might be worth
> > rewording now that we keep them in a separate array.
>
> I did read over that as part of the search I did for things that need
> to be updated, but I didn't see the need to adjust anything since the
> text doesn't talk about where the members are stored. The only thing
> I see as a hint to that is the final sentence.
>
> If the README is light on documentation about where members are
> stored, do we really need to start detailing that because of this
> change? I've tried to be fairly comprehensive about where members are
> stored in the header comment for struct EquivalenceClass. Wouldn't
> stating something similar in the README just be duplicating that? I
> always think of the READMEs as more of an overview on how things fit
> together with some high-level theory. I think talking about data
> structures might be a bit too much detail.
>
> I'm happy to view wording suggestions if you think we need to detail
> this further. Maybe there's something that can be adjusted without
> going into too much depth.
Fair point that the current text isn't wrong (as Tom says) -- and we
do have the storage details in the struct comment already as you say.
Still, maybe a tiny tweak to the last line could help steer readers
right without diving into storage. How about:
Most operations on EquivalenceClasses should ignore child members,
which are stored separately from normal members.
No big deal either way -- just throwing it out there.
--
Thanks, Amit Langote
