Remove unnecessary code in dependency_is_compatible_expression().
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 14 Mar 2023 15:10:45 +0000 (11:10 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 14 Mar 2023 15:10:45 +0000 (11:10 -0400)
Scanning the expression for compatible Vars isn't really necessary,
because the subsequent match against StatisticExtInfo entries will
eliminate expressions containing other Vars just fine.  Moreover,
this code hadn't stopped to think about what to do with
PlaceHolderVars or Aggrefs in the clause; and at least for the PHV
case, that demonstrably leads to failures.  Rather than work out
whether it's reasonable to ignore those, let's just remove the
whole stanza.

Per report from Richard Guo.  Back-patch to v14 where this code
was added.

Discussion: https://postgr.es/m/CAMbWs48Mmvm-acGevXuwpB=g5JMqVSL6i9z5UaJyLGJqa-XPAA@mail.gmail.com

src/backend/statistics/dependencies.c

index a156756b4e1be8e5d28c2d6ad049c4e9deb6c003..edb2e5347d45febf9cb1bc79b34d2ee9c97ad45a 100644 (file)
@@ -1163,13 +1163,12 @@ clauselist_apply_dependencies(PlannerInfo *root, List *clauses,
  *             Determines if the expression is compatible with functional dependencies
  *
  * Similar to dependency_is_compatible_clause, but doesn't enforce that the
- * expression is a simple Var. OTOH we check that there's at least one
- * statistics object matching the expression.
+ * expression is a simple Var.  On success, return the matching statistics
+ * expression into *expr.
  */
 static bool
 dependency_is_compatible_expression(Node *clause, Index relid, List *statlist, Node **expr)
 {
-       List       *vars;
        ListCell   *lc,
                           *lc2;
        Node       *clause_expr;
@@ -1316,29 +1315,8 @@ dependency_is_compatible_expression(Node *clause, Index relid, List *statlist, N
        if (IsA(clause_expr, RelabelType))
                clause_expr = (Node *) ((RelabelType *) clause_expr)->arg;
 
-       vars = pull_var_clause(clause_expr, 0);
-
-       foreach(lc, vars)
-       {
-               Var                *var = (Var *) lfirst(lc);
-
-               /* Ensure Var is from the correct relation */
-               if (var->varno != relid)
-                       return false;
-
-               /* We also better ensure the Var is from the current level */
-               if (var->varlevelsup != 0)
-                       return false;
-
-               /* Also ignore system attributes (we don't allow stats on those) */
-               if (!AttrNumberIsForUserDefinedAttr(var->varattno))
-                       return false;
-       }
-
        /*
-        * Check if we actually have a matching statistics for the expression.
-        *
-        * XXX Maybe this is an overkill. We'll eliminate the expressions later.
+        * Search for a matching statistics expression.
         */
        foreach(lc, statlist)
        {