Support boolean columns in functional-dependency statistics.

There's no good reason that the multicolumn stats stuff shouldn't work on
booleans.  But it looked only for "Var = pseudoconstant" clauses, and it
will seldom find those for boolean Vars, since earlier phases of planning
will fold "boolvar = true" or "boolvar = false" to just "boolvar" or
"NOT boolvar" respectively.  Improve dependencies_clauselist_selectivity()
to recognize such clauses as equivalent to equality restrictions.

This fixes a failure of the extended stats mechanism to apply in a case
reported by Vitaliy Garnashevich.  It's not a complete solution to his
problem because the bitmap-scan costing code isn't consulting extended
stats where it should, but that's surely an independent issue.

In passing, improve some comments, get rid of a NumRelids() test that's
redundant with the preceding bms_membership() test, and fix
dependencies_clauselist_selectivity() so that estimatedclauses actually
is a pure output argument as stated by its API contract.

Back-patch to v10 where this code was introduced.

Discussion: https://postgr.es/m/73a4936d-2814-dc08-ed0c-978f76f435b0@gmail.com

diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index 9756fb83c0c855481be992b41b59fa0d8a806836..ae0f304dba56d4ce11ff0fb071ff7585fa776b37 100644 (file)
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -736,91 +736,104 @@ pg_dependencies_send(PG_FUNCTION_ARGS)
  * dependency_is_compatible_clause
  *             Determines if the clause is compatible with functional dependencies
  *
- * Only OpExprs with two arguments using an equality operator are supported.
- * When returning True attnum is set to the attribute number of the Var within
- * the supported clause.
- *
- * Currently we only support Var = Const, or Const = Var. It may be possible
- * to expand on this later.
+ * Only clauses that have the form of equality to a pseudoconstant, or can be
+ * interpreted that way, are currently accepted.  Furthermore the variable
+ * part of the clause must be a simple Var belonging to the specified
+ * relation, whose attribute number we return in *attnum on success.
  */
 static bool
 dependency_is_compatible_clause(Node *clause, Index relid, AttrNumber *attnum)
 {
        RestrictInfo *rinfo = (RestrictInfo *) clause;
+       Var                *var;
 
        if (!IsA(rinfo, RestrictInfo))
                return false;
 
-       /* Pseudoconstants are not really interesting here. */
+       /* Pseudoconstants are not interesting (they couldn't contain a Var) */
        if (rinfo->pseudoconstant)
                return false;
 
-       /* clauses referencing multiple varnos are incompatible */
+       /* Clauses referencing multiple, or no, varnos are incompatible */
        if (bms_membership(rinfo->clause_relids) != BMS_SINGLETON)
                return false;
 
        if (is_opclause(rinfo->clause))
        {
+               /* If it's an opclause, check for Var = Const or Const = Var. */
                OpExpr     *expr = (OpExpr *) rinfo->clause;
-               Var                *var;
-               bool            varonleft = true;
-               bool            ok;
 
-               /* Only expressions with two arguments are considered compatible. */
+               /* Only expressions with two arguments are candidates. */
                if (list_length(expr->args) != 2)
                        return false;
 
-               /* see if it actually has the right */
-               ok = (NumRelids((Node *) expr) == 1) &&
-                       (is_pseudo_constant_clause(lsecond(expr->args)) ||
-                        (varonleft = false,
-                         is_pseudo_constant_clause(linitial(expr->args))));
-
-               /* unsupported structure (two variables or so) */
-               if (!ok)
+               /* Make sure non-selected argument is a pseudoconstant. */
+               if (is_pseudo_constant_clause(lsecond(expr->args)))
+                       var = linitial(expr->args);
+               else if (is_pseudo_constant_clause(linitial(expr->args)))
+                       var = lsecond(expr->args);
+               else
                        return false;
 
                /*
-                * If it's not "=" operator, just ignore the clause, as it's not
+                * If it's not an "=" operator, just ignore the clause, as it's not
                 * compatible with functional dependencies.
                 *
                 * This uses the function for estimating selectivity, not the operator
                 * directly (a bit awkward, but well ...).
+                *
+                * XXX this is pretty dubious; probably it'd be better to check btree
+                * or hash opclass membership, so as not to be fooled by custom
+                * selectivity functions, and to be more consistent with decisions
+                * elsewhere in the planner.
                 */
                if (get_oprrest(expr->opno) != F_EQSEL)
                        return false;
 
-               var = (varonleft) ? linitial(expr->args) : lsecond(expr->args);
-
+               /* OK to proceed with checking "var" */
+       }
+       else if (not_clause((Node *) rinfo->clause))
+       {
                /*
-                * We may ignore any RelabelType node above the operand.  (There won't
-                * be more than one, since eval_const_expressions() has been applied
-                * already.)
+                * "NOT x" can be interpreted as "x = false", so get the argument and
+                * proceed with seeing if it's a suitable Var.
                 */
-               if (IsA(var, RelabelType))
-                       var = (Var *) ((RelabelType *) var)->arg;
+               var = (Var *) get_notclausearg(rinfo->clause);
+       }
+       else
+       {
+               /*
+                * A boolean expression "x" can be interpreted as "x = true", so
+                * proceed with seeing if it's a suitable Var.
+                */
+               var = (Var *) rinfo->clause;
+       }
 
-               /* We only support plain Vars for now */
-               if (!IsA(var, Var))
-                       return false;
+       /*
+        * We may ignore any RelabelType node above the operand.  (There won't be
+        * more than one, since eval_const_expressions has been applied already.)
+        */
+       if (IsA(var, RelabelType))
+               var = (Var *) ((RelabelType *) var)->arg;
 
-               /* Ensure var is from the correct relation */
-               if (var->varno != relid)
-                       return false;
+       /* We only support plain Vars for now */
+       if (!IsA(var, Var))
+               return false;
 
-               /* we also better ensure the Var is from the current level */
-               if (var->varlevelsup > 0)
-                       return false;
+       /* Ensure Var is from the correct relation */
+       if (var->varno != relid)
+               return false;
 
-               /* Also skip system attributes (we don't allow stats on those). */
-               if (!AttrNumberIsForUserDefinedAttr(var->varattno))
-                       return false;
+       /* We also better ensure the Var is from the current level */
+       if (var->varlevelsup != 0)
+               return false;
 
-               *attnum = var->varattno;
-               return true;
-       }
+       /* Also ignore system attributes (we don't allow stats on those) */
+       if (!AttrNumberIsForUserDefinedAttr(var->varattno))
+               return false;
 
-       return false;
+       *attnum = var->varattno;
+       return true;
 }
 
 /*
@@ -891,12 +904,12 @@ find_strongest_dependency(StatisticExtInfo *stats, MVDependencies *dependencies,
 
 /*
  * dependencies_clauselist_selectivity
- *             Return the estimated selectivity of the given clauses using
- *             functional dependency statistics, or 1.0 if no useful functional
+ *             Return the estimated selectivity of (a subset of) the given clauses
+ *             using functional dependency statistics, or 1.0 if no useful functional
  *             dependency statistic exists.
  *
  * 'estimatedclauses' is an output argument that gets a bit set corresponding
- * to the (zero-based) list index of clauses that are included in the
+ * to the (zero-based) list index of each clause that is included in the
  * estimated selectivity.
  *
  * Given equality clauses on attributes (a,b) we find the strongest dependency
@@ -932,6 +945,9 @@ dependencies_clauselist_selectivity(PlannerInfo *root,
        AttrNumber *list_attnums;
        int                     listidx;
 
+       /* initialize output argument */
+       *estimatedclauses = NULL;
+
        /* check if there's any stats that might be useful for us. */
        if (!has_stats_of_kind(rel->statlist, STATS_EXT_DEPENDENCIES))
                return 1.0;