I looked into bug #8591:
http://www.postgresql.org/message-id/e1vgk41-00050x...@wrigleys.postgresql.org
and was able to reproduce the problem. The proximate cause is that
get_eclass_for_sort_expr is wrong to suppose that it can always create new
equivalence class entries with empty em_nullable_relids; in the case
where the call is coming from initialize_mergeclause_eclasses, it's
essential to use the info supplied in restrictinfo-nullable_relids.
Otherwise, quals deduced from the equivalence class may have wrong
nullable_relids settings, allowing them to be pushed to unsafe places
in 9.2 and later.
This is relatively easy to fix as in the attached patch, but I'm wondering
how risky this is to back-patch. We can deal with the field added to
PlannerInfo by sticking it at the end of the struct in the back branches;
we've done that before and not heard squawks. However, the signature
change for get_eclass_for_sort_expr() would be problematic if any
third-party code is calling that function directly. I'm inclined to think
there probably isn't any such code, since none of the existing calls are
in index-specific code or other places that people would have been likely
to copy and modify. Still, I can't claim that the risk is zero.
We could hack around that in the back branches in more or less ugly ways,
such as by making get_eclass_for_sort_expr() be a wrapper around a new
function. However, it's far from clear what such a wrapper ought to do
--- it could pass NULL for nullable_relids, which would match the existing
behavior, but the odds seem good that the existing behavior might be wrong
for whatever an extension is trying to do. And having the code text
diverge between HEAD and back branches isn't so appetizing anyway.
I'm a bit inclined to take the risk of breaking anything that's calling
get_eclass_for_sort_expr() directly. Thoughts?
regards, tom lane
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 817b149..b39927e 100644
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
*** _outPlannerInfo(StringInfo str, const Pl
*** 1698,1703
--- 1698,1704
WRITE_UINT_FIELD(query_level);
WRITE_NODE_FIELD(plan_params);
WRITE_BITMAPSET_FIELD(all_baserels);
+ WRITE_BITMAPSET_FIELD(nullable_baserels);
WRITE_NODE_FIELD(join_rel_list);
WRITE_INT_FIELD(join_cur_level);
WRITE_NODE_FIELD(init_plans);
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 711b161..baddd34 100644
*** a/src/backend/optimizer/path/equivclass.c
--- b/src/backend/optimizer/path/equivclass.c
*** add_eq_member(EquivalenceClass *ec, Expr
*** 510,515
--- 510,522
* equivalence class it is a member of; if none, optionally build a new
* single-member EquivalenceClass for it.
*
+ * expr is the expression, and nullable_relids is the set of base relids
+ * that are potentially nullable below it. We actually only care about
+ * the set of such relids that are used in the expression; but for caller
+ * convenience, we perform that intersection step here. The caller need
+ * only be sure that nullable_relids doesn't omit any nullable rels that
+ * might appear in the expr.
+ *
* sortref is the SortGroupRef of the originating SortGroupClause, if any,
* or zero if not. (It should never be zero if the expression is volatile!)
*
*** add_eq_member(EquivalenceClass *ec, Expr
*** 538,543
--- 545,551
EquivalenceClass *
get_eclass_for_sort_expr(PlannerInfo *root,
Expr *expr,
+ Relids nullable_relids,
List *opfamilies,
Oid opcintype,
Oid collation,
*** get_eclass_for_sort_expr(PlannerInfo *ro
*** 545,550
--- 553,559
Relids rel,
bool create_it)
{
+ Relids expr_relids;
EquivalenceClass *newec;
EquivalenceMember *newem;
ListCell *lc1;
*** get_eclass_for_sort_expr(PlannerInfo *ro
*** 556,561
--- 565,576
expr = canonicalize_ec_expression(expr, opcintype, collation);
/*
+ * Get the precise set of nullable relids appearing in the expression.
+ */
+ expr_relids = pull_varnos((Node *) expr);
+ nullable_relids = bms_intersect(nullable_relids, expr_relids);
+
+ /*
* Scan through the existing EquivalenceClasses for a match
*/
foreach(lc1, root-eq_classes)
*** get_eclass_for_sort_expr(PlannerInfo *ro
*** 629,636
if (newec-ec_has_volatile sortref == 0) /* should not happen */
elog(ERROR, volatile EquivalenceClass has no sortref);
! newem = add_eq_member(newec, copyObject(expr), pull_varnos((Node *) expr),
! NULL, false, opcintype);
/*
* add_eq_member doesn't check for volatile functions, set-returning
--- 644,651
if (newec-ec_has_volatile sortref == 0) /* should not happen */
elog(ERROR, volatile EquivalenceClass has no sortref);
!
