On Wed, Apr 9, 2025 at 2:21 PM David Rowley <dgrowle...@gmail.com> wrote: > On Wed, 9 Apr 2025 at 17:09, Amit Langote <amitlangot...@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
