Clear out the reminants of EXTEND INDEX

Started by Martijn van Oosterhoutabout 25 years ago13 messagespatches
Jump to latest
#1Martijn van Oosterhout
kleptog@svana.org

Wow Tom, I'm impressed. You took the entire index system and turned it on
it's head, cleaned up most of my ugly bits and managed to remove most of the
stuff relating to extend in the process.

You missed a few spots though, so here a little patch to clear out the rest
(although maybe you're doing them as we speak, can't tell).

You also took care of my next idea, which was to allow IS NULL in the index
predicate.

Anyway, keep it up :)
--
Martijn van Oosterhout <kleptog@svana.org>
http://svana.org/kleptog/

Show quoted text

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.

Attachments:

remove-extend-new.patchtext/plain; charset=us-asciiDownload+0-46
#2Bruce Momjian
bruce@momjian.us
In reply to: Martijn van Oosterhout (#1)
Re: Clear out the reminants of EXTEND INDEX

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.

Wow Tom, I'm impressed. You took the entire index system and turned it on
it's head, cleaned up most of my ugly bits and managed to remove most of the
stuff relating to extend in the process.

You missed a few spots though, so here a little patch to clear out the rest
(although maybe you're doing them as we speak, can't tell).

You also took care of my next idea, which was to allow IS NULL in the index
predicate.

Anyway, keep it up :)
--
Martijn van Oosterhout <kleptog@svana.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.

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#3Bruce Momjian
bruce@momjian.us
In reply to: Martijn van Oosterhout (#1)
Re: Clear out the reminants of EXTEND INDEX

Wow Tom, I'm impressed. You took the entire index system and turned it on
it's head, cleaned up most of my ugly bits and managed to remove most of the
stuff relating to extend in the process.

You missed a few spots though, so here a little patch to clear out the rest
(although maybe you're doing them as we speak, can't tell).

You also took care of my next idea, which was to allow IS NULL in the index
predicate.

Anyway, keep it up :)

I am thinking I should apply this right away before the code changes
again.

Comments?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Martijn van Oosterhout (#1)
Re: Clear out the reminants of EXTEND INDEX

Martijn van Oosterhout <kleptog@svana.org> writes:

You also took care of my next idea, which was to allow IS NULL in the index
predicate.

No I didn't ... or at least, you can build such an index, but you can't
*do* much of anything useful with it. The indxpath.c
predicate-implication code needs work to understand IS NULL and friends
as well as plain operator expressions. Feel free to hack on it.

diff -u -r1.91 nodes.h
--- src/include/nodes/nodes.h	2001/06/19 22:39:12	1.91
+++ src/include/nodes/nodes.h	2001/07/16 09:35:35
@@ -158,7 +158,6 @@
T_DropStmt,
T_TruncateStmt,
T_CommentStmt,
-	T_ExtendStmt,
T_FetchStmt,
T_IndexStmt,
T_ProcedureStmt,

This is not good --- we try to avoid renumbering existing node types.
(Not for any essential reason, perhaps, but why risk breaking anything?)
My practice when removing a node type is to change the nodes.h entry
like this:

T_RemoveOperStmt,
T_RemoveStmt_XXX, /* not used anymore; tag# available */
T_RenameStmt,

Adding the _XXX is sufficient to ensure that any missed uses of the
typecode in the source will yield compile errors.

Otherwise it looks OK.

regards, tom lane

#5Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
Re: Clear out the reminants of EXTEND INDEX
diff -u -r1.91 nodes.h
--- src/include/nodes/nodes.h	2001/06/19 22:39:12	1.91
+++ src/include/nodes/nodes.h	2001/07/16 09:35:35
@@ -158,7 +158,6 @@
T_DropStmt,
T_TruncateStmt,
T_CommentStmt,
-	T_ExtendStmt,
T_FetchStmt,
T_IndexStmt,
T_ProcedureStmt,

This is not good --- we try to avoid renumbering existing node types.
(Not for any essential reason, perhaps, but why risk breaking anything?)
My practice when removing a node type is to change the nodes.h entry
like this:

T_RemoveOperStmt,
T_RemoveStmt_XXX, /* not used anymore; tag# available */
T_RenameStmt,

Adding the _XXX is sufficient to ensure that any missed uses of the
typecode in the source will yield compile errors.

Otherwise it looks OK.

I always rip that stuff out. We do an initdb for every release. What
could break by renumbering those nodes?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#5)
Re: Clear out the reminants of EXTEND INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I always rip that stuff out. We do an initdb for every release. What
could break by renumbering those nodes?

Actually, since we don't store node type numbers on disk (just symbolic
names), there's probably nothing that could break as long as people do a
full recompile when they update. It's no worse than changing the fields
of a widely-used struct, I suppose. I was just being paranoid.

If you want to apply the patch as-is then you might as well remove all
the "XXX"'d items in nodes.h...

regards, tom lane

#7Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#6)
Re: Clear out the reminants of EXTEND INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I always rip that stuff out. We do an initdb for every release. What
could break by renumbering those nodes?

Actually, since we don't store node type numbers on disk (just symbolic
names), there's probably nothing that could break as long as people do a
full recompile when they update. It's no worse than changing the fields
of a widely-used struct, I suppose. I was just being paranoid.

If you want to apply the patch as-is then you might as well remove all
the "XXX"'d items in nodes.h...

OK, patch applied, and I applied the following additional patch to
remove two XXX entries in nodes.h.

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

Attachments:

/bjm/difftext/plainDownload+0-2
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#7)
Re: Clear out the reminants of EXTEND INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I always rip that stuff out. We do an initdb for every release. What
could break by renumbering those nodes?

Actually, since we don't store node type numbers on disk (just symbolic
names), there's probably nothing that could break as long as people do a
full recompile when they update. It's no worse than changing the fields
of a widely-used struct, I suppose. I was just being paranoid.

On third thought, there is some value to preserving the node numbers:
when we see a bug report from the field like

ERROR: ExecEvalExpr: unknown expression type 108

we don't have to resort to excavating back versions of nodes.h to figure
out which node type is meant.

regards, tom lane

#9Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#8)
Re: Clear out the reminants of EXTEND INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I always rip that stuff out. We do an initdb for every release. What
could break by renumbering those nodes?

Actually, since we don't store node type numbers on disk (just symbolic
names), there's probably nothing that could break as long as people do a
full recompile when they update. It's no worse than changing the fields
of a widely-used struct, I suppose. I was just being paranoid.

On third thought, there is some value to preserving the node numbers:
when we see a bug report from the field like

ERROR: ExecEvalExpr: unknown expression type 108

we don't have to resort to excavating back versions of nodes.h to figure
out which node type is meant.

If we do that, we have to only add entries to the end and not put them
with other appropriate symbols, and we have to keep them around forever.
Doesn't seem worth it to me.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#9)
Re: Clear out the reminants of EXTEND INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

If we do that, we have to only add entries to the end and not put them
with other appropriate symbols,

Au contraire: that's exactly why holes were left in the numbering to
start with.

regards, tom lane

#11Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#10)
Re: Clear out the reminants of EXTEND INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

If we do that, we have to only add entries to the end and not put them
with other appropriate symbols,

Au contraire: that's exactly why holes were left in the numbering to
start with.

Yea, but in each group there are items that sort of go together. In the
current XXX discussion, the Remove* items go together, right? If we had
a new Remove item, we should put it with the others, not at the bottom.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#12Bruce Momjian
bruce@momjian.us
In reply to: Martijn van Oosterhout (#1)
Re: Clear out the reminants of EXTEND INDEX

Patch applied.

Wow Tom, I'm impressed. You took the entire index system and turned it on
it's head, cleaned up most of my ugly bits and managed to remove most of the
stuff relating to extend in the process.

You missed a few spots though, so here a little patch to clear out the rest
(although maybe you're doing them as we speak, can't tell).

You also took care of my next idea, which was to allow IS NULL in the index
predicate.

Anyway, keep it up :)
--
Martijn van Oosterhout <kleptog@svana.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.

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#11)
Re: Clear out the reminants of EXTEND INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Yea, but in each group there are items that sort of go together. In the
current XXX discussion, the Remove* items go together, right? If we had
a new Remove item, we should put it with the others, not at the bottom.

That has not been our practice in the past. For the most part we've
added new node types at the ends of their sections. If we were going
to organize the numbering as you suggest then a lot of rearrangement
could be done (and NO, I am NOT advocating that).

regards, tom lane