Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5748f3a

Browse files
committed
Improve predtest.c's internal docs, and enhance its functionality a bit.
Commit b08df9c left things rather poorly documented as far as the exact semantics of "clause_is_check" mode went. Also, that mode did not really work correctly for predicate_refuted_by; although given the lack of specification as to what it should do, as well as the lack of any actual use-case, that's perhaps not surprising. Rename "clause_is_check" to "weak" proof mode, and provide specifications for what it should do. I defined weak refutation as meaning "truth of A implies non-truth of B", which makes it possible to use the mode in the part of relation_excluded_by_constraints that checks for mutually contradictory WHERE clauses. Fix up several places that did things wrong for that definition. (As far as I can see, these errors would only lead to failure-to-prove, not incorrect claims of proof, making them not serious bugs even aside from the fact that v10 contains no use of this mode. So there seems no need for back-patching.) In addition, teach predicate_refuted_by_recurse that it can use predicate_implied_by_recurse after all when processing a strong NOT-clause, so long as it asks for the correct proof strength. This is an optimization that could have been included in commit b08df9c, but wasn't. Also, simplify and generalize the logic that checks for whether nullness of the argument of IS [NOT] NULL would force overall nullness of the predicate or clause. (This results in a change in the partition_prune test's output, as it is now able to prune an all-nulls partition that it did not recognize before.) In passing, in PartConstraintImpliedByRelConstraint, remove bogus conversion of the constraint list to explicit-AND form and then right back again; that accomplished nothing except forcing a useless extra level of recursion inside predicate_implied_by. Discussion: https://postgr.es/m/5983.1520487191@sss.pgh.pa.us
1 parent a63c327 commit 5748f3a

File tree

8 files changed

+364
-163
lines changed

8 files changed

+364
-163
lines changed

src/backend/commands/tablecmds.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13651,10 +13651,11 @@ ComputePartitionAttrs(Relation rel, List *partParams, AttrNumber *partattrs,
1365113651

1365213652
/*
1365313653
* PartConstraintImpliedByRelConstraint
13654-
* Does scanrel's existing constraints imply the partition constraint?
13654+
* Do scanrel's existing constraints imply the partition constraint?
1365513655
*
13656-
* Existing constraints includes its check constraints and column-level
13657-
* NOT NULL constraints and partConstraint describes the partition constraint.
13656+
* "Existing constraints" include its check constraints and column-level
13657+
* NOT NULL constraints. partConstraint describes the partition constraint,
13658+
* in implicit-AND form.
1365813659
*/
1365913660
bool
1366013661
PartConstraintImpliedByRelConstraint(Relation scanrel,
@@ -13724,10 +13725,15 @@ PartConstraintImpliedByRelConstraint(Relation scanrel,
1372413725
make_ands_implicit((Expr *) cexpr));
1372513726
}
1372613727

13727-
if (existConstraint != NIL)
13728-
existConstraint = list_make1(make_ands_explicit(existConstraint));
13729-
13730-
/* And away we go ... */
13728+
/*
13729+
* Try to make the proof. Since we are comparing CHECK constraints, we
13730+
* need to use weak implication, i.e., we assume existConstraint is
13731+
* not-false and try to prove the same for partConstraint.
13732+
*
13733+
* Note that predicate_implied_by assumes its first argument is known
13734+
* immutable. That should always be true for partition constraints, so we
13735+
* don't test it here.
13736+
*/
1373113737
return predicate_implied_by(partConstraint, existConstraint, true);
1373213738
}
1373313739

src/backend/optimizer/util/plancat.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1421,7 +1421,11 @@ relation_excluded_by_constraints(PlannerInfo *root,
14211421
safe_restrictions = lappend(safe_restrictions, rinfo->clause);
14221422
}
14231423

1424-
if (predicate_refuted_by(safe_restrictions, safe_restrictions, false))
1424+
/*
1425+
* We can use weak refutation here, since we're comparing restriction
1426+
* clauses with restriction clauses.
1427+
*/
1428+
if (predicate_refuted_by(safe_restrictions, safe_restrictions, true))
14251429
return true;
14261430

14271431
/*
@@ -1469,6 +1473,9 @@ relation_excluded_by_constraints(PlannerInfo *root,
14691473
* an obvious optimization. Some of the clauses might be OR clauses that
14701474
* have volatile and nonvolatile subclauses, and it's OK to make
14711475
* deductions with the nonvolatile parts.
1476+
*
1477+
* We need strong refutation because we have to prove that the constraints
1478+
* would yield false, not just NULL.
14721479
*/
14731480
if (predicate_refuted_by(safe_constraints, rel->baserestrictinfo, false))
14741481
return true;

0 commit comments

Comments
 (0)