public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Improve node type forward reference
Date: Mon, 14 Oct 2024 09:47:59 +0200
Message-ID: <[email protected]> (raw)
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.
From df3f09a1d12c91595ad0702a17a2a7d884f5193f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 14 Oct 2024 08:52:34 +0200
Subject: [PATCH 1/2] Improve node type forward reference
Instead of using Node *, we can use an incomplete struct. That way,
everything has the correct type and fewer casts are required. This
technique is already used elsewhere in node type definitions.
---
src/backend/commands/createas.c | 2 +-
src/backend/parser/analyze.c | 2 +-
src/include/nodes/primnodes.h | 5 ++---
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 0b629b1f79c..68ec122dbf9 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -133,7 +133,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
if (is_matview)
{
/* StoreViewQuery scribbles on tree, so make a copy */
- Query *query = (Query *) copyObject(into->viewQuery);
+ Query *query = copyObject(into->viewQuery);
StoreViewQuery(intoRelationAddr.objectId, query, false);
CommandCounterIncrement();
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index e901203424d..5e5e0b1c5cf 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3076,7 +3076,7 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
* in the IntoClause because that's where intorel_startup() can
* conveniently get it from.
*/
- stmt->into->viewQuery = (Node *) copyObject(query);
+ stmt->into->viewQuery = copyObject(query);
}
/* represent the command as a utility Query */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index ea47652adb8..f2677b2da1d 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -152,8 +152,7 @@ typedef struct TableFunc
* For CREATE MATERIALIZED VIEW, viewQuery is the parsed-but-not-rewritten
* SELECT Query for the view; otherwise it's NULL. This is irrelevant in
* the query jumbling as CreateTableAsStmt already includes a reference to
- * its own Query, so ignore it. (Although it's actually Query*, we declare
- * it as Node* to avoid a forward reference.)
+ * its own Query, so ignore it.
*/
typedef struct IntoClause
{
@@ -166,7 +165,7 @@ typedef struct IntoClause
OnCommitAction onCommit; /* what do we do at COMMIT? */
char *tableSpaceName; /* table space to use, or NULL */
/* materialized view's SELECT query */
- Node *viewQuery pg_node_attr(query_jumble_ignore);
+ struct Query *viewQuery pg_node_attr(query_jumble_ignore);
bool skipData; /* true for WITH NO DATA */
} IntoClause;
--
2.47.0
From 2cc8cde745140a17c35c17fe3ff10f0a4d5cd12a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 14 Oct 2024 09:00:15 +0200
Subject: [PATCH 2/2] Fix unnecessary casts of copyObject() result
The result is already of the correct type, so these casts don't do
anything.
---
src/backend/commands/trigger.c | 2 +-
src/backend/nodes/nodeFuncs.c | 4 ++--
src/backend/rewrite/rewriteManip.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 3671e82535e..09356e46d16 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -1169,7 +1169,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
* Initialize our fabricated parse node by copying the original
* one, then resetting fields that we pass separately.
*/
- childStmt = (CreateTrigStmt *) copyObject(stmt);
+ childStmt = copyObject(stmt);
childStmt->funcname = NIL;
childStmt->whenClause = NULL;
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 0d00e029f32..f76072228c9 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2996,7 +2996,7 @@ expression_tree_mutator_impl(Node *node,
case T_SortGroupClause:
case T_CTESearchClause:
case T_MergeSupportFunc:
- return (Node *) copyObject(node);
+ return copyObject(node);
case T_WithCheckOption:
{
WithCheckOption *wco = (WithCheckOption *) node;
@@ -3604,7 +3604,7 @@ expression_tree_mutator_impl(Node *node,
break;
case T_PartitionPruneStepCombine:
/* no expression sub-nodes */
- return (Node *) copyObject(node);
+ return copyObject(node);
case T_JoinExpr:
{
JoinExpr *join = (JoinExpr *) node;
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index b20625fbd2b..8f90afb3269 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1715,7 +1715,7 @@ ReplaceVarsFromTargetList_callback(Var *var,
break;
case REPLACEVARS_CHANGE_VARNO:
- var = (Var *) copyObject(var);
+ var = copyObject(var);
var->varno = rcon->nomatch_varno;
/* we leave the syntactic referent alone */
return (Node *) var;
--
2.47.0
Attachments:
[text/plain] 0001-Improve-node-type-forward-reference.patch (2.7K, ../[email protected]/2-0001-Improve-node-type-forward-reference.patch)
download | inline diff:
From df3f09a1d12c91595ad0702a17a2a7d884f5193f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 14 Oct 2024 08:52:34 +0200
Subject: [PATCH 1/2] Improve node type forward reference
Instead of using Node *, we can use an incomplete struct. That way,
everything has the correct type and fewer casts are required. This
technique is already used elsewhere in node type definitions.
---
src/backend/commands/createas.c | 2 +-
src/backend/parser/analyze.c | 2 +-
src/include/nodes/primnodes.h | 5 ++---
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 0b629b1f79c..68ec122dbf9 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -133,7 +133,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
if (is_matview)
{
/* StoreViewQuery scribbles on tree, so make a copy */
- Query *query = (Query *) copyObject(into->viewQuery);
+ Query *query = copyObject(into->viewQuery);
StoreViewQuery(intoRelationAddr.objectId, query, false);
CommandCounterIncrement();
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index e901203424d..5e5e0b1c5cf 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3076,7 +3076,7 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
* in the IntoClause because that's where intorel_startup() can
* conveniently get it from.
*/
- stmt->into->viewQuery = (Node *) copyObject(query);
+ stmt->into->viewQuery = copyObject(query);
}
/* represent the command as a utility Query */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index ea47652adb8..f2677b2da1d 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -152,8 +152,7 @@ typedef struct TableFunc
* For CREATE MATERIALIZED VIEW, viewQuery is the parsed-but-not-rewritten
* SELECT Query for the view; otherwise it's NULL. This is irrelevant in
* the query jumbling as CreateTableAsStmt already includes a reference to
- * its own Query, so ignore it. (Although it's actually Query*, we declare
- * it as Node* to avoid a forward reference.)
+ * its own Query, so ignore it.
*/
typedef struct IntoClause
{
@@ -166,7 +165,7 @@ typedef struct IntoClause
OnCommitAction onCommit; /* what do we do at COMMIT? */
char *tableSpaceName; /* table space to use, or NULL */
/* materialized view's SELECT query */
- Node *viewQuery pg_node_attr(query_jumble_ignore);
+ struct Query *viewQuery pg_node_attr(query_jumble_ignore);
bool skipData; /* true for WITH NO DATA */
} IntoClause;
--
2.47.0
[text/plain] 0002-Fix-unnecessary-casts-of-copyObject-result.patch (2.3K, ../[email protected]/3-0002-Fix-unnecessary-casts-of-copyObject-result.patch)
download | inline diff:
From 2cc8cde745140a17c35c17fe3ff10f0a4d5cd12a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Mon, 14 Oct 2024 09:00:15 +0200
Subject: [PATCH 2/2] Fix unnecessary casts of copyObject() result
The result is already of the correct type, so these casts don't do
anything.
---
src/backend/commands/trigger.c | 2 +-
src/backend/nodes/nodeFuncs.c | 4 ++--
src/backend/rewrite/rewriteManip.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 3671e82535e..09356e46d16 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -1169,7 +1169,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
* Initialize our fabricated parse node by copying the original
* one, then resetting fields that we pass separately.
*/
- childStmt = (CreateTrigStmt *) copyObject(stmt);
+ childStmt = copyObject(stmt);
childStmt->funcname = NIL;
childStmt->whenClause = NULL;
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 0d00e029f32..f76072228c9 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2996,7 +2996,7 @@ expression_tree_mutator_impl(Node *node,
case T_SortGroupClause:
case T_CTESearchClause:
case T_MergeSupportFunc:
- return (Node *) copyObject(node);
+ return copyObject(node);
case T_WithCheckOption:
{
WithCheckOption *wco = (WithCheckOption *) node;
@@ -3604,7 +3604,7 @@ expression_tree_mutator_impl(Node *node,
break;
case T_PartitionPruneStepCombine:
/* no expression sub-nodes */
- return (Node *) copyObject(node);
+ return copyObject(node);
case T_JoinExpr:
{
JoinExpr *join = (JoinExpr *) node;
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index b20625fbd2b..8f90afb3269 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1715,7 +1715,7 @@ ReplaceVarsFromTargetList_callback(Var *var,
break;
case REPLACEVARS_CHANGE_VARNO:
- var = (Var *) copyObject(var);
+ var = copyObject(var);
var->varno = rcon->nomatch_varno;
/* we leave the syntactic referent alone */
return (Node *) var;
--
2.47.0
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Improve node type forward reference
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox