Subject: Redundant check of em_is_child

From: Amit Langote <[email-masked]>
Date: Fri, 19 May 2017 16:46:39 +0900

In match_eclasses_to_foreign_key_col(), there is this:

            if (em->em_is_child)
                continue;       /* ignore children here */

ISTM, it might as well be:

            Assert(!em->em_is_child);    /* no children yet */

That's because, I think it's still too early in query_planner() to be
expecting any child EC members.

Thanks,
Amit
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 67bd760fb4..735bd7fdc6 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -2017,8 +2017,7 @@ match_eclasses_to_foreign_key_col(PlannerInfo *root,
 			EquivalenceMember *em = (EquivalenceMember *) lfirst(lc2);
 			Var		   *var;
 
-			if (em->em_is_child)
-				continue;		/* ignore children here */
+			Assert(!em->em_is_child);	/* no children yet */
 
 			/* EM must be a Var, possibly with RelabelType */
 			var = (Var *) em->em_expr;

-- 
Sent via pgsql-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

---

From: Robert Haas <[email-masked]>
Date: Thu, 22 Jun 2017 11:00:58 -0400

> expecting any child EC members.

I'm not sure there's really any benefit to this change.  In the
future, somebody might want to use the function from someplace later
on in the planner.  If the logic as-written would work correctly in
that case now, I can't see why we should turn it into an assertion
failure instead.  This isn't really costing us anything, is it?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Amit Langote <[email-masked]>
Date: Fri, 23 Jun 2017 09:35:19 +0900

> failure instead.  This isn't really costing us anything, is it?

Not much perhaps.  Just thought it might be an oversight (and potentially
a source of confusion today to someone trying to understand this code),
but no harm in leaving it the way it is, as you say, so that someone in
the future doesn't have to worry about checking here.

Sorry for the noise.

Thanks,
Amit