[HACKERS] Re: Extended statistics is not working on Vars hidden under a RelabelType

2017-10-12 Thread Alvaro Herrera
David Rowley wrote:

> The reason Tomas coded it the way it was coded is due to the fact that
> there's already code that works exactly the same way in
> clauselist_selectivity(). Personally, I don't particularly like that
> code, but I'd rather not invent a new way to do the same thing.

I pushed your original fix.  I still maintain that this should be
written differently, but I'm not going to try to change that in a hurry
and together with a bugfix.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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


[HACKERS] Re: Extended statistics is not working on Vars hidden under a RelabelType

2017-10-12 Thread Alvaro Herrera
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 Herrerahttps://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;
-   boolvaronleft = true;
-   boolok;
 
-   /* 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
+

[HACKERS] Re: Extended statistics is not working on Vars hidden under a RelabelType

2017-10-10 Thread Tomas Vondra
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.

regards

-- 
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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