Re: [PATCH] Partial indicies again - Mailing list pgsql-general

From Tom Lane
Subject Re: [PATCH] Partial indicies again
Date
Msg-id 23407.994721133@sss.pgh.pa.us
Whole thread Raw
In response to [PATCH] Partial indicies again  (Martijn van Oosterhout <kleptog@svana.org>)
Responses Re: [PATCH] Partial indicies again  (Martijn van Oosterhout <kleptog@svana.org>)
List pgsql-general
Martijn van Oosterhout <kleptog@svana.org> writes:
> * As Tom Lane pointed out, what to do with EXTEND INDEX. I've decided to
>   remove it since there is no defined syntax for it and you cannot make an
>   index more restrictive with it. Since the names of indicies don't matter,
>   you may as well drop the index and make a new one if you want to change
>   the predicate.

I can imagine ways to deal with the make-it-more-restrictive problem
(one creative way is to mark the index lossy if we can't prove that the
old predicate implies the new one).  But I'd favor just removing the
statement, instead.  It doesn't seem useful enough to be worth the
substantial baggage.  (In addition to the statement itself, there's
cruft in each instance of index build to deal with EXTEND.  All that
stuff should come out IMHO.)

> * Changes to the cost estimation. This is much better than before but still
>   needs some refining since index_selectivity != rel_selectivity. Feedback
>   welcomed.

I suspect you need to run cnfify after and-ing together the predicate
and index quals.  clauselist_selectivity assumes it's working on
cnf'ified expressions.

> * Make it pg_dump-able. I've tried to extract the expression out the of
>   system tables by using stringToNode and deparse_expression but it doesn't
>   seem to work. I keep getting the error: "get_names_for_var: bogus
>   varlevelsup 0". Anyone know what's going on? See attachment <<expr.c>>.

You didn't provide context in which to decode Var references.  See
deparse_context_for().  The SQL function will need to take a relation
name (or OID? not sure which is preferable) as well as the compiled
predicate.  Btw, don't set forceprefix true; we don't need that if
there's only going to be one relation involved.

> * VACUUM still complains about tuple count mismatch. Anyone have a good idea
>   on how to deal with this?

I'd be inclined to just weaken VACUUM's test from "=" to "<=" whenever
the index has a predicate.

> Oh, and it appears I accidently changed the allowed syntax a bit. You used
> to have to qualify each field in the partial index predicate with the name
> of the relation. That's no longer required.

This is definitely an improvement; we should consider the unqualified
way to be the default, recommended style.  You should change the
regression test to exercise both forms.

            regards, tom lane

pgsql-general by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: Interfacing with MSAccess
Next
From: Joseph Shraibman
Date:
Subject: Re: Backups WAS: 2 gig file size limit