Index USING in pg_dump

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

This is possible because btree is the default. TODO item is:
* Remove USING clause from pg_get_indexdef() if index is btree (Bruce)

I do not think this is necessary or helpful. The only possible
reason to change it would be if we thought btree might someday
not be the default index type --- but no such change is on the
horizon. And if one was, you've just embedded special knowledge
about btree in yet one more place...

regards, tom lane

Tom Lane wrote:

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

This is possible because btree is the default. TODO item is:
* Remove USING clause from pg_get_indexdef() if index is btree (Bruce)

I do not think this is necessary or helpful. The only possible
reason to change it would be if we thought btree might someday
not be the default index type --- but no such change is on the
horizon. And if one was, you've just embedded special knowledge
about btree in yet one more place...

Yes, but it doesn't look like the way they created it. Very few use
USING in there queries. Why show it in pg_dump output?

-- 
  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

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

Yes, but it doesn't look like the way they created it.

(a) And you know that how? (b) Are we also supposed to preserve
spacing, keyword case, etc? Not much of an argument...

regards, tom lane

Tom Lane wrote:

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

Yes, but it doesn't look like the way they created it.

(a) And you know that how? (b) Are we also supposed to preserve
spacing, keyword case, etc? Not much of an argument...

Well, the USING part was confusing people because they didn't even know
we had other index types. It is just an attempt to clean up pg_dump
output to be clearer. One change I did make is to add a
DEFAULT_INDEX_TYPE macro and replace "btree" with the use of that macro
in a few places.

Here is a new patch. I am moving the discussion to patches because of
the patch attachment.

How is this? Comments from others?

-- 
  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

On Fri, Mar 08, 2002 at 11:07:57AM -0500, Bruce Momjian wrote:

Tom Lane wrote:

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

This is possible because btree is the default. TODO item is:
* Remove USING clause from pg_get_indexdef() if index is btree (Bruce)

I do not think this is necessary or helpful. The only possible
reason to change it would be if we thought btree might someday
not be the default index type --- but no such change is on the
horizon. And if one was, you've just embedded special knowledge
about btree in yet one more place...

Yes, but it doesn't look like the way they created it.

Why is this relevant?

Very few use
USING in there queries. Why show it in pg_dump output?

I agree with Tom: this seems like a waste of time, and may even be worse
than the current pg_dump output. The type of the index is "btree"; by
assuming that Pg happens to default to "btree", you're just making the
process of index restoration more fragile.

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

Neil Conway wrote:

On Fri, Mar 08, 2002 at 11:07:57AM -0500, Bruce Momjian wrote:

Tom Lane wrote:

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

This is possible because btree is the default. TODO item is:
* Remove USING clause from pg_get_indexdef() if index is btree (Bruce)

I do not think this is necessary or helpful. The only possible
reason to change it would be if we thought btree might someday
not be the default index type --- but no such change is on the
horizon. And if one was, you've just embedded special knowledge
about btree in yet one more place...

Yes, but it doesn't look like the way they created it.

Why is this relevant?

Very few use
USING in there queries. Why show it in pg_dump output?

I agree with Tom: this seems like a waste of time, and may even be worse
than the current pg_dump output. The type of the index is "btree"; by
assuming that Pg happens to default to "btree", you're just making the
process of index restoration more fragile.

OK, how about this patch? It just creates a macro so btree is a clear
default. It no longer affects pg_dump.

-- 
  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

Yes, but it doesn't look like the way they created it.

(a) And you know that how? (b) Are we also supposed to preserve
spacing, keyword case, etc? Not much of an argument...

I think the initial idea was rather to try to use most common
syntax where possible, and USING is not very common :-)

Andreas

OK, there was a tie in votes of whether we should remove "USING btree"
from pg_dump, so it isn't worth changing it. I will apply the following
patch that adds DEFAULT_INDEX_TYPE so things are clearer.

---------------------------------------------------------------------------

Bruce Momjian wrote:

Neil Conway wrote:

On Fri, Mar 08, 2002 at 11:07:57AM -0500, Bruce Momjian wrote:

Tom Lane wrote:

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

This is possible because btree is the default. TODO item is:
* Remove USING clause from pg_get_indexdef() if index is btree (Bruce)

I do not think this is necessary or helpful. The only possible
reason to change it would be if we thought btree might someday
not be the default index type --- but no such change is on the
horizon. And if one was, you've just embedded special knowledge
about btree in yet one more place...

Yes, but it doesn't look like the way they created it.

Why is this relevant?

Very few use
USING in there queries. Why show it in pg_dump output?

I agree with Tom: this seems like a waste of time, and may even be worse
than the current pg_dump output. The type of the index is "btree"; by
assuming that Pg happens to default to "btree", you're just making the
process of index restoration more fragile.

OK, how about this patch? It just creates a macro so btree is a clear
default. It no longer affects pg_dump.

-- 
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

Index: src/backend/parser/analyze.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.217
diff -c -r1.217 analyze.c
*** src/backend/parser/analyze.c	6 Mar 2002 06:09:51 -0000	1.217
--- src/backend/parser/analyze.c	8 Mar 2002 16:25:59 -0000
***************
*** 16,21 ****
--- 16,22 ----
#include "access/heapam.h"
#include "catalog/catname.h"
#include "catalog/heap.h"
+ #include "catalog/index.h"
#include "catalog/pg_index.h"
#include "catalog/pg_type.h"
#include "nodes/makefuncs.h"
***************
*** 1049,1055 ****
index->idxname = NULL;		/* will set it later */

index->relname = cxt->relname;
! index->accessMethod = "btree";
index->indexParams = NIL;
index->whereClause = NULL;

--- 1050,1056 ----
index->idxname = NULL;		/* will set it later */

index->relname = cxt->relname;
! index->accessMethod = DEFAULT_INDEX_TYPE;
index->indexParams = NIL;
index->whereClause = NULL;

Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.287
diff -c -r2.287 gram.y
*** src/backend/parser/gram.y	7 Mar 2002 16:35:35 -0000	2.287
--- src/backend/parser/gram.y	8 Mar 2002 16:26:03 -0000
***************
*** 51,56 ****
--- 51,57 ----
#include <ctype.h>

#include "access/htup.h"
+ #include "catalog/index.h"
#include "catalog/pg_type.h"
#include "nodes/params.h"
#include "nodes/parsenodes.h"
***************
*** 2539,2545 ****
;

access_method_clause: USING access_method { $$ = $2; }
! | /*EMPTY*/ { $$ = "btree"; }
;

index_params:  index_list						{ $$ = $1; }
--- 2540,2547 ----
;

access_method_clause: USING access_method { $$ = $2; }
! /* If btree changes as our default, update pg_get_indexdef() */
! | /*EMPTY*/ { $$ = DEFAULT_INDEX_TYPE; }
;

index_params:  index_list						{ $$ = $1; }
Index: src/include/catalog/index.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/index.h,v
retrieving revision 1.44
diff -c -r1.44 index.h
*** src/include/catalog/index.h	19 Feb 2002 20:11:19 -0000	1.44
--- src/include/catalog/index.h	8 Mar 2002 16:26:12 -0000
***************
*** 18,23 ****
--- 18,24 ----
#include "catalog/pg_index.h"
#include "nodes/execnodes.h"

+ #define DEFAULT_INDEX_TYPE "btree"

/* Typedef for callback function for IndexBuildHeapScan */
typedef void (*IndexBuildCallback) (Relation index,

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  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