Tomas Vondra wrote:
> On 10/10/2017 05:03 AM, David Rowley wrote:
> > Basically, $subject is causing us not to properly find matching
> > extended stats in this case.
> > 
> > The attached patch fixes it.
> > 
> > The following test cases is an example of the misbehaviour. Note
> > rows=1 vs rows=98 in the Gather node.
> 
> Thanks for noticing this. The patch seems fine to me.

I propose this slightly larger change.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/statistics/dependencies.c 
b/src/backend/statistics/dependencies.c
index 2e7c0ad6ba..0a51639d9d 100644
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -736,12 +736,9 @@ 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.
+ * Returns true, and sets *attnum to the attribute number of the Var in the
+ * clause, if the clause is compatible with functional dependencies on the
+ * given relation.  Otherwise, return false.
  */
 static bool
 dependency_is_compatible_clause(Node *clause, Index relid, AttrNumber *attnum)
@@ -762,39 +759,47 @@ dependency_is_compatible_clause(Node *clause, Index 
relid, AttrNumber *attnum)
        if (is_opclause(rinfo->clause))
        {
                OpExpr     *expr = (OpExpr *) rinfo->clause;
+               Node       *left;
+               Node       *right;
                Var                *var;
-               bool            varonleft = true;
-               bool            ok;
 
-               /* Only expressions with two arguments are considered 
compatible. */
+               /* Only expressions with two arguments are considered 
compatible ... */
                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)
+               /* ... and only if they operate on a single relation */
+               if (NumRelids((Node *) expr) != 1)
                        return false;
 
                /*
-                * If it's not "=" 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 ...).
+                * See which one is the pseudo-constant one, if any. If neither 
is, the
+                * clause is not compatible.
                 */
-               if (get_oprrest(expr->opno) != F_EQSEL)
+               left = (Node *) linitial(expr->args);
+               right = (Node *) lsecond(expr->args);
+
+               if (IsA(left, Var) || IsA(left, RelabelType))
+               {
+                       if (!is_pseudo_constant_clause(right))
+                               return false;
+                       var = (Var *) left;
+               }
+               else if (IsA(right, Var) || IsA(right, RelabelType))
+               {
+                       if (!is_pseudo_constant_clause(left))
+                               return false;
+                       var = (Var *) right;
+               }
+               else
                        return false;
 
-               var = (varonleft) ? linitial(expr->args) : lsecond(expr->args);
-
-               /* 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)
@@ -808,6 +813,16 @@ dependency_is_compatible_clause(Node *clause, Index relid, 
AttrNumber *attnum)
                if (!AttrNumberIsForUserDefinedAttr(var->varattno))
                        return false;
 
+               /*
+                * If it's not "=" 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 ...).
+                */
+               if (get_oprrest(expr->opno) != F_EQSEL)
+                       return false;
+
                *attnum = var->varattno;
                return true;
        }
-- 
2.11.0

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

Reply via email to