Proposed patch for bug #3921 - Mailing list pgsql-patches

Attached is a proposed patch for bug #3921, which complained that CREATE
TABLE LIKE INCLUDING INDEXES fails inappropriately for non-superusers.

There are two parts to the patch: the first follows Greg Starks' opinion
that explicitly specifying the current database's default tablespace
shouldn't be an error at all, and so the permissions checks during
table/index creation are suppressed when tablespaceId ==
MyDatabaseTableSpace.  (Note that the assign hooks for
default_tablespace and temp_tablespaces already behaved this way, so we
were not exactly being consistent anyhow.)  I couldn't find anyplace in
the documentation that specifies the old behavior, so no docs changes.

The second part fixes the LIKE INCLUDING INDEXES code to default the
index tablespace specification when the source index is in the
database's default tablespace.  This would provide an alternative
way of fixing the bug if anyone doesn't like the first part.  With the
first part it's redundant, but seems worth doing anyway to avoid the
overhead of looking up the tablespace name and then converting it back
to OID in the typical case.

Also there is a correction of an obsolete comment in parsenodes.h, which
should be applied in any case.

An issue that this patch doesn't address is what happens if the source
index is in a non-default tablespace that the current user does not have
CREATE permission for.  With both current CVS HEAD and this patch, that
will result in an error.  Is that okay?  We could imagine making
parse_utilcmd.c check the permissions and default the tablespace
specification if no good, but that behavior seems a bit peculiar to me.
Command semantics don't normally change based on whether you have
permissions or not.

An entirely different tack we could take is to adopt the position
that LIKE INCLUDING INDEXES shouldn't copy index tablespaces at all,
but I didn't hear anyone favoring that approach in the bug discussion.

            regards, tom lane

Index: src/backend/commands/indexcmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v
retrieving revision 1.170
diff -c -r1.170 indexcmds.c
*** src/backend/commands/indexcmds.c    9 Jan 2008 21:52:36 -0000    1.170
--- src/backend/commands/indexcmds.c    3 Feb 2008 02:32:14 -0000
***************
*** 215,221 ****
      }

      /* Check permissions except when using database's default */
!     if (OidIsValid(tablespaceId))
      {
          AclResult    aclresult;

--- 215,221 ----
      }

      /* Check permissions except when using database's default */
!     if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
      {
          AclResult    aclresult;

Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.241
diff -c -r1.241 tablecmds.c
*** src/backend/commands/tablecmds.c    30 Jan 2008 19:46:48 -0000    1.241
--- src/backend/commands/tablecmds.c    3 Feb 2008 02:32:15 -0000
***************
*** 340,346 ****
      }

      /* Check permissions except when using database's default */
!     if (OidIsValid(tablespaceId))
      {
          AclResult    aclresult;

--- 340,346 ----
      }

      /* Check permissions except when using database's default */
!     if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
      {
          AclResult    aclresult;

Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.302
diff -c -r1.302 execMain.c
*** src/backend/executor/execMain.c    1 Jan 2008 19:45:49 -0000    1.302
--- src/backend/executor/execMain.c    3 Feb 2008 02:32:15 -0000
***************
*** 2594,2600 ****
      }

      /* Check permissions except when using the database's default space */
!     if (OidIsValid(tablespaceId))
      {
          AclResult    aclresult;

--- 2594,2600 ----
      }

      /* Check permissions except when using the database's default space */
!     if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
      {
          AclResult    aclresult;

Index: src/backend/parser/parse_utilcmd.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_utilcmd.c,v
retrieving revision 2.8
diff -c -r2.8 parse_utilcmd.c
*** src/backend/parser/parse_utilcmd.c    1 Jan 2008 19:45:51 -0000    2.8
--- src/backend/parser/parse_utilcmd.c    3 Feb 2008 02:32:15 -0000
***************
*** 767,773 ****
      index = makeNode(IndexStmt);
      index->relation = cxt->relation;
      index->accessMethod = pstrdup(NameStr(amrec->amname));
!     index->tableSpace = get_tablespace_name(source_idx->rd_node.spcNode);
      index->unique = idxrec->indisunique;
      index->primary = idxrec->indisprimary;
      index->concurrent = false;
--- 767,776 ----
      index = makeNode(IndexStmt);
      index->relation = cxt->relation;
      index->accessMethod = pstrdup(NameStr(amrec->amname));
!     if (OidIsValid(idxrelrec->reltablespace))
!         index->tableSpace = get_tablespace_name(idxrelrec->reltablespace);
!     else
!         index->tableSpace = NULL;
      index->unique = idxrec->indisunique;
      index->primary = idxrec->indisprimary;
      index->concurrent = false;
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.358
diff -c -r1.358 parsenodes.h
*** src/include/nodes/parsenodes.h    1 Jan 2008 19:45:58 -0000    1.358
--- src/include/nodes/parsenodes.h    3 Feb 2008 02:32:15 -0000
***************
*** 1537,1543 ****
      char       *idxname;        /* name of new index, or NULL for default */
      RangeVar   *relation;        /* relation to build index on */
      char       *accessMethod;    /* name of access method (eg. btree) */
!     char       *tableSpace;        /* tablespace, or NULL to use parent's */
      List       *indexParams;    /* a list of IndexElem */
      List       *options;        /* options from WITH clause */
      Node       *whereClause;    /* qualification (partial-index predicate) */
--- 1537,1543 ----
      char       *idxname;        /* name of new index, or NULL for default */
      RangeVar   *relation;        /* relation to build index on */
      char       *accessMethod;    /* name of access method (eg. btree) */
!     char       *tableSpace;        /* tablespace, or NULL for default */
      List       *indexParams;    /* a list of IndexElem */
      List       *options;        /* options from WITH clause */
      Node       *whereClause;    /* qualification (partial-index predicate) */