Improve node type forward reference
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.
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);
--
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.