Horton

Thread

Commits

  1. Fix unnecessary casts of copyObject() result

  2. Improve node type forward reference

  1. Improve node type forward reference

    Peter Eisentraut <peter@eisentraut.org> — 2024-10-14T07:47:59Z

    This is a small code simplification.

In primnodes.h, we have

     typedef struct IntoClause
     {
         ...
         /* materialized view's SELECT query */
         Node       *viewQuery ...;

with the comment

     /* (Although it's actually Query*, we declare
      * it as Node* to avoid a forward reference.)

But we can do this better by using an incomplete struct, like

         struct Query *viewQuery ...;

That way, everything has the correct type and fewer casts are required. 
This technique is already used elsewhere in node type definitions.

The second patch just removes some more unnecessary casts around 
copyObject() that I found while working on this.

  2. Re: Improve node type forward reference

    Nathan Bossart <nathandbossart@gmail.com> — 2024-10-14T21:28:34Z

    On Mon, Oct 14, 2024 at 09:47:59AM +0200, Peter Eisentraut wrote:
> But we can do this better by using an incomplete struct, like
> 
>         struct Query *viewQuery ...;
> 
> That way, everything has the correct type and fewer casts are required. This
> technique is already used elsewhere in node type definitions.

I noticed that the examples in parsenodes.h are for structs defined within
the same file.  If the struct is defined in a separate file, I guess you
might need to include another header file wherever it is used, but that
doesn't seem too bad.

> The second patch just removes some more unnecessary casts around
> copyObject() that I found while working on this.

LGTM

-- 
nathan

  3. Re: Improve node type forward reference

    Peter Eisentraut <peter@eisentraut.org> — 2024-10-15T07:02:48Z

    On 14.10.24 23:28, Nathan Bossart wrote:
> On Mon, Oct 14, 2024 at 09:47:59AM +0200, Peter Eisentraut wrote:
>> But we can do this better by using an incomplete struct, like
>>
>>          struct Query *viewQuery ...;
>>
>> That way, everything has the correct type and fewer casts are required. This
>> technique is already used elsewhere in node type definitions.
> 
> I noticed that the examples in parsenodes.h are for structs defined within
> the same file.  If the struct is defined in a separate file, I guess you
> might need to include another header file wherever it is used, but that
> doesn't seem too bad.

No, you can leave the struct incomplete.  You only need to provide its 
full definition (= include the other header file) if you actually want 
to access the struct's fields.

  4. Re: Improve node type forward reference

    Tender Wang <tndrwang@gmail.com> — 2024-10-15T09:03:05Z

    Peter Eisentraut <peter@eisentraut.org> 于2024年10月15日周二 15:02写道：

> On 14.10.24 23:28, Nathan Bossart wrote:
> > On Mon, Oct 14, 2024 at 09:47:59AM +0200, Peter Eisentraut wrote:
> >> But we can do this better by using an incomplete struct, like
> >>
> >>          struct Query *viewQuery ...;
> >>
> >> That way, everything has the correct type and fewer casts are required.
> This
> >> technique is already used elsewhere in node type definitions.
>

Agree.  The attached patches LGTM.


> >
> > I noticed that the examples in parsenodes.h are for structs defined
> within
> > the same file.  If the struct is defined in a separate file, I guess you
> > might need to include another header file wherever it is used, but that
> > doesn't seem too bad.
>
> No, you can leave the struct incomplete.  You only need to provide its
> full definition (= include the other header file) if you actually want
> to access the struct's fields.
>
>
Yeah.   There are many examples. For example as below:
in struct RelOptInfos,

/* use "struct FdwRoutine" to avoid including fdwapi.h here */
struct FdwRoutine *fdwroutine pg_node_attr(read_write_ignore);

-- 
Thanks,
Tender Wang

  5. Re: Improve node type forward reference

    Nathan Bossart <nathandbossart@gmail.com> — 2024-10-15T14:43:41Z

    On Tue, Oct 15, 2024 at 09:02:48AM +0200, Peter Eisentraut wrote:
> On 14.10.24 23:28, Nathan Bossart wrote:
>> On Mon, Oct 14, 2024 at 09:47:59AM +0200, Peter Eisentraut wrote:
>> > But we can do this better by using an incomplete struct, like
>> > 
>> >          struct Query *viewQuery ...;
>> > 
>> > That way, everything has the correct type and fewer casts are required. This
>> > technique is already used elsewhere in node type definitions.
>> 
>> I noticed that the examples in parsenodes.h are for structs defined within
>> the same file.  If the struct is defined in a separate file, I guess you
>> might need to include another header file wherever it is used, but that
>> doesn't seem too bad.
> 
> No, you can leave the struct incomplete.  You only need to provide its full
> definition (= include the other header file) if you actually want to access
> the struct's fields.

That's what I figured.  This one LGTM, too, then.

-- 
nathan

  6. Re: Improve node type forward reference

    Peter Eisentraut <peter@eisentraut.org> — 2024-10-17T06:41:09Z

    On 15.10.24 16:43, Nathan Bossart wrote:
> On Tue, Oct 15, 2024 at 09:02:48AM +0200, Peter Eisentraut wrote:
>> On 14.10.24 23:28, Nathan Bossart wrote:
>>> On Mon, Oct 14, 2024 at 09:47:59AM +0200, Peter Eisentraut wrote:
>>>> But we can do this better by using an incomplete struct, like
>>>>
>>>>           struct Query *viewQuery ...;
>>>>
>>>> That way, everything has the correct type and fewer casts are required. This
>>>> technique is already used elsewhere in node type definitions.
>>>
>>> I noticed that the examples in parsenodes.h are for structs defined within
>>> the same file.  If the struct is defined in a separate file, I guess you
>>> might need to include another header file wherever it is used, but that
>>> doesn't seem too bad.
>>
>> No, you can leave the struct incomplete.  You only need to provide its full
>> definition (= include the other header file) if you actually want to access
>> the struct's fields.
> 
> That's what I figured.  This one LGTM, too, then.

Committed, thanks.