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

Commit 0f0deb7

Browse files
committed
Improve predtest.c's handling of cases with NULL-constant inputs.
Currently, if operator_predicate_proof() is given an operator clause like "something op NULL", it just throws up its hands and reports it can't prove anything. But we can often do better than that, if the operator is strict, because then we know that the clause returns NULL overall. Depending on whether we're trying to prove or refute something, and whether we need weak or strong semantics for NULL, this may be enough to prove the implication, especially when we rely on the standard rule that "false implies anything". In particular, this lets us do something useful with questions like "does X IN (1,3,5,NULL) imply X <= 5?" The null entry in the IN list can effectively be ignored for this purpose, but the proof rules were not previously smart enough to deduce that. This patch is by me, but it owes something to previous work by Amit Langote to try to solve problems of the form mentioned. Thanks also to Emre Hasegeli and Ashutosh Bapat for review. Discussion: https://postgr.es/m/3bad48fc-f257-c445-feeb-8a2b2fb622ba@lab.ntt.co.jp
1 parent 27ba260 commit 0f0deb7

File tree

4 files changed

+46
-23
lines changed

4 files changed

+46
-23
lines changed

src/backend/optimizer/util/predtest.c

+44-15
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ static Node *extract_not_arg(Node *clause);
100100
static Node *extract_strong_not_arg(Node *clause);
101101
static bool clause_is_strict_for(Node *clause, Node *subexpr);
102102
static bool operator_predicate_proof(Expr *predicate, Node *clause,
103-
bool refute_it);
103+
bool refute_it, bool weak);
104104
static bool operator_same_subexprs_proof(Oid pred_op, Oid clause_op,
105105
bool refute_it);
106106
static bool operator_same_subexprs_lookup(Oid pred_op, Oid clause_op,
@@ -1137,7 +1137,7 @@ predicate_implied_by_simple_clause(Expr *predicate, Node *clause,
11371137
}
11381138

11391139
/* Else try operator-related knowledge */
1140-
return operator_predicate_proof(predicate, clause, false);
1140+
return operator_predicate_proof(predicate, clause, false, weak);
11411141
}
11421142

11431143
/*----------
@@ -1232,7 +1232,7 @@ predicate_refuted_by_simple_clause(Expr *predicate, Node *clause,
12321232
}
12331233

12341234
/* Else try operator-related knowledge */
1235-
return operator_predicate_proof(predicate, clause, true);
1235+
return operator_predicate_proof(predicate, clause, true, weak);
12361236
}
12371237

12381238

@@ -1498,9 +1498,8 @@ static const StrategyNumber BT_refute_table[6][6] = {
14981498
* values, since then the operators aren't being given identical inputs. But
14991499
* we only support that for btree operators, for which we can assume that all
15001500
* non-null inputs result in non-null outputs, so that it doesn't matter which
1501-
* two non-null constants we consider. Currently the code below just reports
1502-
* "proof failed" if either constant is NULL, but in some cases we could be
1503-
* smarter (and that likely would require checking strong vs. weak proofs).
1501+
* two non-null constants we consider. If either constant is NULL, we have
1502+
* to think harder, but sometimes the proof still works, as explained below.
15041503
*
15051504
* We can make proofs involving several expression forms (here "foo" and "bar"
15061505
* represent subexpressions that are identical according to equal()):
@@ -1528,7 +1527,8 @@ static const StrategyNumber BT_refute_table[6][6] = {
15281527
* and we dare not make deductions with those.
15291528
*/
15301529
static bool
1531-
operator_predicate_proof(Expr *predicate, Node *clause, bool refute_it)
1530+
operator_predicate_proof(Expr *predicate, Node *clause,
1531+
bool refute_it, bool weak)
15321532
{
15331533
OpExpr *pred_opexpr,
15341534
*clause_opexpr;
@@ -1675,17 +1675,46 @@ operator_predicate_proof(Expr *predicate, Node *clause, bool refute_it)
16751675
* We have two identical subexpressions, and two other subexpressions that
16761676
* are not identical but are both Consts; and we have commuted the
16771677
* operators if necessary so that the Consts are on the right. We'll need
1678-
* to compare the Consts' values. If either is NULL, fail.
1679-
*
1680-
* Future work: in some cases the desired proof might hold even with NULL
1681-
* constants. But beware that we've not yet identified the operators as
1682-
* btree ops, so for instance it'd be quite unsafe to assume they are
1683-
* strict without checking.
1678+
* to compare the Consts' values. If either is NULL, we can't do that, so
1679+
* usually the proof fails ... but in some cases we can claim success.
16841680
*/
1685-
if (pred_const->constisnull)
1686-
return false;
16871681
if (clause_const->constisnull)
1682+
{
1683+
/* If clause_op isn't strict, we can't prove anything */
1684+
if (!op_strict(clause_op))
1685+
return false;
1686+
1687+
/*
1688+
* At this point we know that the clause returns NULL. For proof
1689+
* types that assume truth of the clause, this means the proof is
1690+
* vacuously true (a/k/a "false implies anything"). That's all proof
1691+
* types except weak implication.
1692+
*/
1693+
if (!(weak && !refute_it))
1694+
return true;
1695+
1696+
/*
1697+
* For weak implication, it's still possible for the proof to succeed,
1698+
* if the predicate can also be proven NULL. In that case we've got
1699+
* NULL => NULL which is valid for this proof type.
1700+
*/
1701+
if (pred_const->constisnull && op_strict(pred_op))
1702+
return true;
1703+
/* Else the proof fails */
1704+
return false;
1705+
}
1706+
if (pred_const->constisnull)
1707+
{
1708+
/*
1709+
* If the pred_op is strict, we know the predicate yields NULL, which
1710+
* means the proof succeeds for either weak implication or weak
1711+
* refutation.
1712+
*/
1713+
if (weak && op_strict(pred_op))
1714+
return true;
1715+
/* Else the proof fails */
16881716
return false;
1717+
}
16891718

16901719
/*
16911720
* Lookup the constant-comparison operator using the system catalogs and

src/test/modules/test_predtest/expected/test_predtest.out

+1-2
Original file line numberDiff line numberDiff line change
@@ -781,13 +781,12 @@ w_i_holds | f
781781
s_r_holds | f
782782
w_r_holds | f
783783

784-
-- XXX ideally, we could prove this case too, for strong implication
785784
select * from test_predtest($$
786785
select x <= 5, x in (1,3,5,null)
787786
from integers
788787
$$);
789788
-[ RECORD 1 ]-----+--
790-
strong_implied_by | f
789+
strong_implied_by | t
791790
weak_implied_by | f
792791
strong_refuted_by | f
793792
weak_refuted_by | f

src/test/modules/test_predtest/sql/test_predtest.sql

-1
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,6 @@ select x <= 5, x in (1,3,5,7)
306306
from integers
307307
$$);
308308

309-
-- XXX ideally, we could prove this case too, for strong implication
310309
select * from test_predtest($$
311310
select x <= 5, x in (1,3,5,null)
312311
from integers

src/test/regress/expected/inherit.out

+1-5
Original file line numberDiff line numberDiff line change
@@ -1739,11 +1739,7 @@ explain (costs off) select * from list_parted where a = 'ab' or a in (null, 'cd'
17391739
Append
17401740
-> Seq Scan on part_ab_cd
17411741
Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY ('{NULL,cd}'::text[])))
1742-
-> Seq Scan on part_ef_gh
1743-
Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY ('{NULL,cd}'::text[])))
1744-
-> Seq Scan on part_null_xy
1745-
Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY ('{NULL,cd}'::text[])))
1746-
(7 rows)
1742+
(3 rows)
17471743

17481744
explain (costs off) select * from list_parted where a = 'ab';
17491745
QUERY PLAN

0 commit comments

Comments
 (0)