Обсуждение: Re: [PERFORM] Feature request: smarter use of conditional indexes
On 3/3/04 6:53 PM, Tom Lane wrote:
> John Siracusa <siracusa@mindspring.com> writes:
>> Given an index like this:
>> CREATE UNIQUE INDEX i1 ON t1 (c1) WHERE c1 IS NOT NULL;
>> and a query like this:
>> SELECT * FROM t1 WHERE c1 = 123;
>> I'd like the planner to be smart enough to use an index scan using
>> i1.
>
> Send a patch ;-)
(Originally sent to the wrong list and in the wrong format...sorry :)
How does this look? It seems to do what I want without horribly
breaking anything as far as I can tell. I ran "make check" and got the
same result as I did before my changes (5 failures in OS X 10.3.2).
But then, I also got the same result when I wasn't even checking to
make sure that both clauses were looking at the same variable :) I'm
not sure how to add a test for this particular change either.
Index: src/backend/optimizer/path/indxpath.c
===================================================================
RCS file:
/projects/cvsroot/pgsql-server/src/backend/optimizer/path/indxpath.c,v
retrieving revision 1.156
diff -c -r1.156 indxpath.c
*** src/backend/optimizer/path/indxpath.c 7 Jan 2004 22:02:48
-0000 1.156
--- src/backend/optimizer/path/indxpath.c 7 Mar 2004 03:49:17 -0000
***************
*** 1030,1036 ****
--- 1030,1060 ----
* the test operator will always be strict.
*/
if (!is_opclause(predicate))
+ {
+ /* One last chance: "var = const" or "const = var" implies "var is
not null" */
+ if (IsA(predicate, NullTest) &&
+ ((NullTest *) predicate)->nulltesttype == IS_NOT_NULL &&
+ is_opclause(clause) && op_strict(((OpExpr *) clause)->opno) &&
+ length(((OpExpr *) clause)->args) == 2)
+ {
+ leftop = get_leftop((Expr *) clause);
+ rightop = get_rightop((Expr *) clause);
+
+ /* One of the two arguments must be a constant */
+ if (IsA(rightop, Const))
+ clause_var = leftop;
+ else if (IsA(leftop, Const))
+ clause_var = rightop;
+ else
+ return false;
+
+ /* Finally, make sure "var" is the same var in both clauses */
+ if (equal(((NullTest *) predicate)->arg, clause_var))
+ return true;
+ }
+
return false;
+ }
leftop = get_leftop(predicate);
rightop = get_rightop(predicate);
if (rightop == NULL)
John Siracusa wrote: > How does this look? It seems to do what I want without horribly > breaking anything as far as I can tell. I ran "make check" and got the > same result as I did before my changes (5 failures in OS X 10.3.2). Uh, you get test failures on OS X 10.3.2? On my OS X machine with the latest CVS HEAD code, the tests run without any failures. -Neil
Neil Conway <neilc@samurai.com> writes:
> Uh, you get test failures on OS X 10.3.2? On my OS X machine with the
> latest CVS HEAD code, the tests run without any failures.
I was wondering about that too. I get one failure --- a one-liner
zero-versus-minus-zero difference in geometry.out. Anything more seems
cause for suspicion.
regards, tom lane
On 3/6/04 11:07 PM, Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: >> Uh, you get test failures on OS X 10.3.2? On my OS X machine with the >> latest CVS HEAD code, the tests run without any failures. > > I was wondering about that too. I get one failure --- a one-liner > zero-versus-minus-zero difference in geometry.out. Anything more seems > cause for suspicion. I know, I get that same 1 failure in 7.4.1 on my Mac at work which is (seemingly) identical to my Mac at home. I just deleted my CVS repository and then re-checked-out the whole thing, rebuilt it (no problems there, as usual), and ran make check again. This time I got 9 failures! When I got the 5 failures with an unmodified CVS checkout earlier, I didn't even bother to look at the diffs because I just accepted that as my baseline (it is CVS, after all, and things are in flux). But this time, a quick look at the diffs showed lines like this in the regression diffs: ! psql: could not fork new process for connection: Resource temporarily unavailable I then recalled that my Mac at work has all sorts of kernel params and resource limits tweaked to make Postgres happier. Just running "unlimit" in my shell was all I needed to do to make "make check" work at home, and I got the same old single geometry test failure (-0 vs. 0) both with and without the patch. Yay. -John
John Siracusa <siracusa@mindspring.com> writes:
> When I got the 5 failures with an unmodified CVS checkout earlier, I didn't
> even bother to look at the diffs because I just accepted that as my baseline
> (it is CVS, after all, and things are in flux).
Just for the record, we don't consider regression failures as status quo
even in CVS tip. People aren't supposed to commit changes that break
regression. If you see a failure appear that wasn't there before, by
all means report it immediately --- it's easiest to fix portability bugs
while the relevant changes are still fresh in mind.
regards, tom lane
John Siracusa <siracusa@mindspring.com> writes:
> How does this look? It seems to do what I want without horribly
> breaking anything as far as I can tell.
Not a bad effort for a backend newbie ;-). It was lacking in comments,
and upon inspection I thought it could be generalized a little. I've
applied the attached modification, which will recognize strict operators
and functions of any number of arguments.
regards, tom lane
*** src/backend/optimizer/path/indxpath.c.orig Wed Jan 7 17:02:48 2004
--- src/backend/optimizer/path/indxpath.c Sun Mar 7 00:13:27 2004
***************
*** 965,988 ****
};
! /*
* pred_test_simple_clause
* Does the "predicate inclusion test" for a "simple clause" predicate
* and a "simple clause" restriction.
*
! * We have two strategies for determining whether one simple clause
! * implies another. A simple and general way is to see if they are
! * equal(); this works for any kind of expression. (Actually, there
! * is an implied assumption that the functions in the expression are
! * immutable, ie dependent only on their input arguments --- but this
! * was checked for the predicate by CheckPredicate().)
*
! * Our other way works only for (binary boolean) operators that are
! * in some btree operator class. We use the above operator implication
! * table to be able to derive implications between nonidentical clauses.
*
! * Eventually, rtree operators could also be handled by defining an
! * appropriate "RT_implic_table" array.
*/
static bool
pred_test_simple_clause(Expr *predicate, Node *clause)
--- 965,1002 ----
};
! /*----------
* pred_test_simple_clause
* Does the "predicate inclusion test" for a "simple clause" predicate
* and a "simple clause" restriction.
*
! * We have three strategies for determining whether one simple clause
! * implies another:
*
! * A simple and general way is to see if they are equal(); this works for any
! * kind of expression. (Actually, there is an implied assumption that the
! * functions in the expression are immutable, ie dependent only on their input
! * arguments --- but this was checked for the predicate by CheckPredicate().)
*
! * When the predicate is of the form "foo IS NOT NULL", we can conclude that
! * the predicate is implied if the clause is a strict operator or function
! * that has "foo" as an input. In this case the clause must yield NULL when
! * "foo" is NULL, which we can take as equivalent to FALSE because we know
! * we are within an AND/OR subtree of a WHERE clause. (Again, "foo" is
! * already known immutable, so the clause will certainly always fail.)
! *
! * Our other way works only for binary boolean opclauses of the form
! * "foo op constant", where "foo" is the same in both clauses. The operators
! * and constants can be different but the operators must be in the same btree
! * operator class. We use the above operator implication table to be able to
! * derive implications between nonidentical clauses. (Note: "foo" is known
! * immutable, and constants are surely immutable, and we assume that operators
! * that are in btree opclasses are immutable, so there's no need to do extra
! * mutability checks in this case either.)
! *
! * Eventually, rtree operators could also be handled by defining an
! * appropriate "RT_implic_table" array.
! *----------
*/
static bool
pred_test_simple_clause(Expr *predicate, Node *clause)
***************
*** 1019,1024 ****
--- 1033,1055 ----
/* First try the equal() test */
if (equal((Node *) predicate, clause))
return true;
+
+ /* Next try the IS NOT NULL case */
+ if (predicate && IsA(predicate, NullTest) &&
+ ((NullTest *) predicate)->nulltesttype == IS_NOT_NULL)
+ {
+ Expr *nonnullarg = ((NullTest *) predicate)->arg;
+
+ if (is_opclause(clause) &&
+ member(nonnullarg, ((OpExpr *) clause)->args) &&
+ op_strict(((OpExpr *) clause)->opno))
+ return true;
+ if (is_funcclause(clause) &&
+ member(nonnullarg, ((FuncExpr *) clause)->args) &&
+ func_strict(((FuncExpr *) clause)->funcid))
+ return true;
+ return false; /* we can't succeed below... */
+ }
/*
* Can't do anything more unless they are both binary opclauses with a
On 3/7/04 12:48 AM, Tom Lane wrote: > John Siracusa <siracusa@mindspring.com> writes: >> How does this look? It seems to do what I want without horribly >> breaking anything as far as I can tell. > > Not a bad effort for a backend newbie ;-). It was lacking in comments, > and upon inspection I thought it could be generalized a little. I was trying to avoid catastrophe by confining myself to the exact case I wanted to fix :) But even if I wasn't, I didn't understand the full meaning of "strict" operators and functions. Actually, I'm still not sure how you can conclude that foo is not null simply by seeing func(..., foo, ...) where func() is strict. Do strict functions and operators simply not allow null args? Anyway, the world of typedefs and macros is definitely not my cup of tea. Here's hoping that my next performance enhancement suggestion will be complex enough that no one will entertain the thought of asking me for a patch... ;) -John
John Siracusa <siracusa@mindspring.com> writes:
> ... Actually, I'm still not sure how you
> can conclude that foo is not null simply by seeing func(..., foo, ...) where
> func() is strict. Do strict functions and operators simply not allow null
> args?
That was what the comments were about ;-). A "strict" operator is one
that always returns NULL if any input is NULL. This does guarantee that
the WHERE condition will fail, but it takes a bit of analysis to see
that the implication is valid.
> Here's hoping that my next performance enhancement suggestion will be
> complex enough that no one will entertain the thought of asking me for a
> patch... ;)
I had quite the opposite motivation in mind in asking you to prototype
this ;-) ...
regards, tom lane