Add explicit initialization for all PlannerGlobal fields

Started by Richard Guo8 months ago3 messages
#1Richard Guo
guofenglinux@gmail.com
1 attachment(s)

While adding a new field to PlannerGlobal in another patch, I noticed
that although most fields are explicitly initialized, a few are not.
This doesn't cause any functional issues, since makeNode() zeroes all
fields by default. However, the inconsistency stood out to me, so I
wrote the attached patch to explicitly initialize the remaining fields
for clarity and consistency.

Does this seem worthwhile? Or should we simply rely on makeNode() for
zero-initialization and consider this unnecessary?

Thanks
Richard

Attachments:

v1-0001-Add-explicit-initialization-for-all-PlannerGlobal.patchapplication/octet-stream; name=v1-0001-Add-explicit-initialization-for-all-PlannerGlobal.patchDownload
From 56a5b6267e51543ed373966f3b3f519e52dc87ce Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Tue, 13 May 2025 16:44:44 +0900
Subject: [PATCH v1] Add explicit initialization for all PlannerGlobal fields

---
 src/backend/optimizer/plan/planner.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index beafac8c0b0..49ad6e83578 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -326,10 +326,14 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
 	glob->subroots = NIL;
 	glob->rewindPlanIDs = NULL;
 	glob->finalrtable = NIL;
+	glob->allRelids = NULL;
+	glob->prunableRelids = NULL;
 	glob->finalrteperminfos = NIL;
 	glob->finalrowmarks = NIL;
 	glob->resultRelations = NIL;
+	glob->firstResultRels = NIL;
 	glob->appendRelations = NIL;
+	glob->partPruneInfos = NIL;
 	glob->relationOids = NIL;
 	glob->invalItems = NIL;
 	glob->paramExecTypes = NIL;
@@ -338,6 +342,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
 	glob->lastPlanNodeId = 0;
 	glob->transientPlan = false;
 	glob->dependsOnRole = false;
+	glob->partition_directory = NULL;
 
 	/*
 	 * Assess whether it's feasible to use parallel mode for this query. We
-- 
2.43.0

#2David Rowley
dgrowleyml@gmail.com
In reply to: Richard Guo (#1)
Re: Add explicit initialization for all PlannerGlobal fields

On Tue, 13 May 2025 at 04:03, Richard Guo <guofenglinux@gmail.com> wrote:

While adding a new field to PlannerGlobal in another patch, I noticed
that although most fields are explicitly initialized, a few are not.
This doesn't cause any functional issues, since makeNode() zeroes all
fields by default. However, the inconsistency stood out to me, so I
wrote the attached patch to explicitly initialize the remaining fields
for clarity and consistency.

Does this seem worthwhile? Or should we simply rely on makeNode() for
zero-initialization and consider this unnecessary?

These should be zeroed explicitly.

David

#3Richard Guo
guofenglinux@gmail.com
In reply to: David Rowley (#2)
Re: Add explicit initialization for all PlannerGlobal fields

On Tue, May 13, 2025 at 8:26 PM David Rowley <dgrowleyml@gmail.com> wrote:

On Tue, 13 May 2025 at 04:03, Richard Guo <guofenglinux@gmail.com> wrote:

While adding a new field to PlannerGlobal in another patch, I noticed
that although most fields are explicitly initialized, a few are not.
This doesn't cause any functional issues, since makeNode() zeroes all
fields by default. However, the inconsistency stood out to me, so I
wrote the attached patch to explicitly initialize the remaining fields
for clarity and consistency.

Does this seem worthwhile? Or should we simply rely on makeNode() for
zero-initialization and consider this unnecessary?

These should be zeroed explicitly.

Thank you for confirming. I've pushed this patch to master.

Thanks
Richard