public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tomas Vondra <[email protected]>
Subject: [PATCH 02/10] review
Date: Tue, 16 Mar 2021 17:29:26 +0100

---
 src/backend/optimizer/path/allpaths.c  | 2 ++
 src/backend/optimizer/plan/initsplan.c | 9 ++++++++-
 src/backend/optimizer/util/plancat.c   | 3 +++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 37b4223adb..fc1a3a68a2 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1065,8 +1065,10 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
 				/* Whole row is not null, so must be same for child */
 				childrel->notnullattrs = bms_add_member(childrel->notnullattrs,
 														attno - FirstLowInvalidHeapAttributeNumber);
+				/* XXX shouldn't this be a continue, instead of a break? */
 				break;
 			}
+			/* XXX isn't this missing 'else'? */
 			if (attno < 0 )
 				/* no need to translate system column */
 				child_attno = attno;
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index d27167dc76..4ef876cf7b 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -829,7 +829,14 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 		{
 			Node	   *qual = (Node *) lfirst(l);
 
-			/* Set the not null info now */
+			/* Set the not null info now
+			 *
+			 * XXX Why now? Why is this the right place to do this? Does it need
+			 * to happen before distribute_qual_to_rels, for example?
+			 *
+			 * XXX Not clear to me why this looks at non-nullable vars? Shouldn't
+			 * we already have the bitmap built from atnums (from get_relation_info)?
+			 */
 			ListCell	*lc;
 			List		*non_nullable_vars = find_nonnullable_vars(qual);
 			foreach(lc, non_nullable_vars)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index eebabcfccf..cd703e41ba 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -481,6 +481,9 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 	if (inhparent && relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 		set_relation_partition_info(root, rel, relation);
 
+	/*
+	 * Build information about which attributes are marked as NOT NULL.
+	 */
 	Assert(rel->notnullattrs == NULL);
 	for(i = 0; i < relation->rd_att->natts; i++)
 	{
-- 
2.30.2


--------------F495F6B18672582269F00FF9
Content-Type: text/x-patch; charset=UTF-8;
 name="0003-Introduce-UniqueKey-attributes-on-RelOptInf-20210317.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0003-Introduce-UniqueKey-attributes-on-RelOptInf-20210317.pa";
 filename*1="tch"

view thread (53+ messages)  latest in thread

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: [PATCH 02/10] review
  In-Reply-To: <no-message-id-1880629@localhost>

* 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