Re: plan shape work

On Sun, Sep 21, 2025 at 5:52 PM Junwang Zhao <zhjwpku@gmail.com> wrote:
>
> Hi Robert,
>
> On Fri, Sep 19, 2025 at 1:23 AM Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > On Tue, Sep 16, 2025 at 11:27 AM Robert Haas <robertmhaas@gmail.com> wrote:
> > > I'm inclined to change the code like this to fix it:
> > >
> > >     char       *replacement_type = "???";
> > >
> > > ...in the hopes of still producing a warning here if somebody adds
> > > another label to the enum.
> >
> > Done in this version. I've also now gone back and rebased the rest of
> > the patches as well, so this email includes all 7 patches instead of
> > just the first 3. To recall, my goal for this CF was to get 1-4
> > committed.
> >
> > --
> > Robert Haas
> > EDB: http://www.enterprisedb.com
>
> I have a question about the following changes:
>
> - splan = plan;
>   if (IsA(plan, Result))
>   {
>   Result    *rplan = (Result *) plan;
>
> - if (rplan->plan.lefttree == NULL &&
> - rplan->resconstantqual == NULL)
> - splan = NULL;
> + gplan->plan.lefttree = NULL;
> + gplan->relids = rplan->relids;
> + gplan->result_type = rplan->result_type;
>   }
>
> You set gplan->relids and gplan->result_type, but at the end you
> returned &gplan->plan, what's the point of setting these two fields?

After further study, this should not be a problem, since the address of
&gplan->plan is the same as gplan, but the code is a little bit confusing
at first glance, I think *return (Plan *) gplan* is easier to understand
but I don't insist ;)

>
>
> --
> Regards
> Junwang Zhao



--
Regards
Junwang Zhao