Thread: Re: Improve node type forward reference
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
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.
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);
struct FdwRoutine *fdwroutine pg_node_attr(read_write_ignore);
Thanks,
Tender Wang
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
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.