[PATCH] To remove EXTEND INDEX

Hi,

As promised, here is a patch to remove all support for the (non-functional
anyway) EXTEND INDEX statement. See earlier emails for more explanations.

Note, this patch makes it as if it never existed. So, if you think some of
the code may be useful, now is the time to speak up! :)

Since this patch conflicts with my currently pending patch for partial
indices, I put it here for review. I will submit a new patch to -patches
once the other is in.

http://svana.org/kleptog/pgsql/remove-extend.patch
--
Martijn van Oosterhout <kleptog(at)svana(dot)org>
http://svana.org/kleptog/
> It would be nice if someone came up with a certification system that
> actually separated those who can barely regurgitate what they crammed over
> the last few weeks from those who command secret ninja networking powers.

> Note, this patch makes it as if it never existed. So, if you think some of
> the code may be useful, now is the time to speak up! :)

Shouldn't this conversation be happening on the -hackers list? TIA

- Thomas

> > Note, this patch makes it as if it never existed. So, if you think some of
> > the code may be useful, now is the time to speak up! :)
>
> Shouldn't this conversation be happening on the -hackers list? TIA

Actually, because it had a patch attached, it should go to patches,
right?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> > > Note, this patch makes it as if it never existed. So, if you think some of
> > > the code may be useful, now is the time to speak up! :)
> > Shouldn't this conversation be happening on the -hackers list? TIA
> Actually, because it had a patch attached, it should go to patches,
> right?

imho, no. Because there needs to be a discussion about whether to remove
code from the tree, and whether that code may be useful for something
else.

-patches is designed to take actual patch files (to reduce bandwidth on
-hackers) but not to host planning discussions. If this had been a
simple patch without feature changes or other larger ramifications, then
it is more clearly a patch-only topic.

- Thomas

> > > > Note, this patch makes it as if it never existed. So, if you think some of
> > > > the code may be useful, now is the time to speak up! :)
> > > Shouldn't this conversation be happening on the -hackers list? TIA
> > Actually, because it had a patch attached, it should go to patches,
> > right?
>
> imho, no. Because there needs to be a discussion about whether to remove
> code from the tree, and whether that code may be useful for something
> else.
>
> -patches is designed to take actual patch files (to reduce bandwidth on
> -hackers) but not to host planning discussions. If this had been a
> simple patch without feature changes or other larger ramifications, then
> it is more clearly a patch-only topic.

What some people do is split the patch text with the actual patch. I
don't like it sometimes, but I hesistate to post big patches to hachers,
even if they require discussion.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> > > > Note, this patch makes it as if it never existed. So, if you think some of
> > > > the code may be useful, now is the time to speak up! :)
> > > Shouldn't this conversation be happening on the -hackers list? TIA
> > Actually, because it had a patch attached, it should go to patches,
> > right?
>
> imho, no. Because there needs to be a discussion about whether to remove
> code from the tree, and whether that code may be useful for something
> else.
>
> -patches is designed to take actual patch files (to reduce bandwidth on
> -hackers) but not to host planning discussions. If this had been a
> simple patch without feature changes or other larger ramifications, then
> it is more clearly a patch-only topic.

Sorry. I thought the posting had an attached patch. I now see it is a
URL. I was wrong.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Let's drop the meta-discussions and cut to the chase: given that we are
about to re-enable partial indexes, should we try to make EXTEND INDEX
work too, or just remove it?

The idea of EXTEND INDEX is to allow replacement of a partial index's
predicate. However, the implementation only supports weakening the
predicate, ie, it can only add tuples to the index not remove them.
The index's predicate is actually turned into (old predicate OR new
predicate), which seems counterintuitive to me.

I am not sure that EXTEND INDEX is actually broken. I originally
thought that the new predicate would replace the old, which would be
wrong --- but I now see the OR-ing behavior in UpdateIndexPredicate, so
it's not necessarily busted. The question is whether the feature has
enough usefulness to be worth supporting and documenting forevermore.
You can accomplish the same things, and more, by dropping the index and
building a new one; what's more, at least in the btree case building a
new one is likely to be much faster (the EXTEND code has to do retail
insertion, not a SORT-based build).

So, is it worth expending any effort on EXTEND INDEX? It seems to me
that it's a fair amount of code bulk and complexity for very very
marginal return. I'd like to simplify the index AM API by getting
rid of the concept.

regards, tom lane

> Let's drop the meta-discussions and cut to the chase: given that we are
> about to re-enable partial indexes, should we try to make EXTEND INDEX
> work too, or just remove it?
>
> The idea of EXTEND INDEX is to allow replacement of a partial index's
> predicate. However, the implementation only supports weakening the
> predicate, ie, it can only add tuples to the index not remove them.
> The index's predicate is actually turned into (old predicate OR new
> predicate), which seems counterintuitive to me.
>
> I am not sure that EXTEND INDEX is actually broken. I originally
> thought that the new predicate would replace the old, which would be
> wrong --- but I now see the OR-ing behavior in UpdateIndexPredicate, so
> it's not necessarily busted. The question is whether the feature has
> enough usefulness to be worth supporting and documenting forevermore.
> You can accomplish the same things, and more, by dropping the index and
> building a new one; what's more, at least in the btree case building a
> new one is likely to be much faster (the EXTEND code has to do retail
> insertion, not a SORT-based build).
>
> So, is it worth expending any effort on EXTEND INDEX? It seems to me
> that it's a fair amount of code bulk and complexity for very very
> marginal return. I'd like to simplify the index AM API by getting
> rid of the concept.

We don't let people add columns to an existing index so I don't see why
we should have EXTEND INDEX unless index twiddling is more common with
partial indexes.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

On Fri, Jul 13, 2001 at 05:49:56PM -0400, Tom Lane wrote:
> Let's drop the meta-discussions and cut to the chase: given that we are
> about to re-enable partial indexes, should we try to make EXTEND INDEX
> work too, or just remove it?

Just a few clarifications:

* The reason it didn't go to -hackers was because I wasn't subscribed to it
and hence couldn't post to it. The only reason I can now is because I
subscribed (nopost) about 2 minutes ago.

* I discussed this with Tom Lane on -general a few days ago. I'm not sure
how many people saw that though. Are most of the people on -hackers
subscribed to -general as well?

* I agree with Tom's assertion that it's an awful lot of complexity for such
a marginal gain. Look at the size of the patch and the fact that it has all
been useless for the last few years.

* I didn't send it to -patches because it's not ready yet.

* Only posted a URL, not the patch itself. Sorry for the confusion.

Tom actually suggested doing this at the same time as re-enabling partial
indices but I favoured a separate patch considering the large number of
scattered changes.

Anyway, is there a concensus, or shall I forget the whole thing?

--
Martijn van Oosterhout <kleptog(at)svana(dot)org>
http://svana.org/kleptog/
> It would be nice if someone came up with a certification system that
> actually separated those who can barely regurgitate what they crammed over
> the last few weeks from those who command secret ninja networking powers.

On Fri, Jul 13, 2001 at 06:34:22PM -0400, Bruce Momjian wrote:
> > Let's drop the meta-discussions and cut to the chase: given that we are
> > about to re-enable partial indexes, should we try to make EXTEND INDEX
> > work too, or just remove it?
>
> We don't let people add columns to an existing index so I don't see why
> we should have EXTEND INDEX unless index twiddling is more common with
> partial indexes.

We don't allow people currently to fiddle with indices at all. I don't
understand the origin of EXTEND INDEX since I can't think of a situation
where it would actually be useful.

--
Martijn van Oosterhout <kleptog(at)svana(dot)org>
http://svana.org/kleptog/
> It would be nice if someone came up with a certification system that
> actually separated those who can barely regurgitate what they crammed over
> the last few weeks from those who command secret ninja networking powers.

> * I agree with Tom's assertion that it's an awful lot of complexity for such
> a marginal gain. Look at the size of the patch and the fact that it has all
> been useless for the last few years.
>
> * I didn't send it to -patches because it's not ready yet.
>
> * Only posted a URL, not the patch itself. Sorry for the confusion.
>
> Tom actually suggested doing this at the same time as re-enabling partial
> indices but I favoured a separate patch considering the large number of
> scattered changes.
>
> Anyway, is there a concensus, or shall I forget the whole thing?

I vote for removing the feature. Removing stuff of doubtful usefulness
is a big gain.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> We don't let people add columns to an existing index so I don't see why
> we should have EXTEND INDEX unless index twiddling is more common with
> partial indexes.
>

Nothing is common with partial indexes at the moment -- the feature is
not currently implemented, and I don't think other databases adopted the
idea.

From memory (*), Stonebraker's original intention for partial indexes
was that they would be used with really large tables in a situation
where you would might to process the table incrementally, a chunk at a
time, an example might be archiving historical data based on date. You
only want to archive information older than a certain date, so you use a
partial index predicated on t < t_0. You then do your archive processing
on those tuples, delete the tuples from the table, and extend the
predicate forward by an interval in aticipation of the next archiving
cycle.

The example is not perfect, but I think that it indicates what the
original author's were thinking. You also have to ask yourself when
would this approach be better than just indexing the whole table, and
use the predicate in the query qualification.

Bernie

(*) The partial indexes are mentioned briefly in one of the Stonebraker
papers. Sorry, I don't have an exact reference, but it is probably in
one of the Stonebraker publications referenced by

http://techdocs.postgresql.org/oresources.php

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> Hi,
>
> As promised, here is a patch to remove all support for the (non-functional
> anyway) EXTEND INDEX statement. See earlier emails for more explanations.
>
> Note, this patch makes it as if it never existed. So, if you think some of
> the code may be useful, now is the time to speak up! :)
>
> Since this patch conflicts with my currently pending patch for partial
> indices, I put it here for review. I will submit a new patch to -patches
> once the other is in.
>
> http://svana.org/kleptog/pgsql/remove-extend.patch
> --
> Martijn van Oosterhout <kleptog(at)svana(dot)org>
> http://svana.org/kleptog/
> > It would be nice if someone came up with a certification system that
> > actually separated those who can barely regurgitate what they crammed over
> > the last few weeks from those who command secret ninja networking powers.
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026