Proposed patch for bug #3921

Started by Tom Laneover 18 years ago3 messagespatches
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

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

#2Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#1)
Re: Proposed patch for bug #3921

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

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?

I was going to say "we could add a hint suggesting how to specify the
tablespace" but as you pointed out earlier there really isn't a way to specify
the tablespace.

Hm, looking at the grammar we already have something like this for
constraints. We could add an OptConsTableSpace after INCLUDING INDEXES. I just
tested it and it didn't cause any conflicts which makes sense since like
options appear in the column list.

So the syntax would be

CREATE TABLE foo (..., LIKE bar INCLUDING INDEXES USING INDEX TABLESPACE foo_ts, ...)

Kind of verbose but nice that it's the same syntax as constraints.

Not sure how easy it would be to shoehorn into t he like processing, I could
look at that if you want.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: Proposed patch for bug #3921

Gregory Stark <stark@enterprisedb.com> writes:

So the syntax would be

CREATE TABLE foo (..., LIKE bar INCLUDING INDEXES USING INDEX TABLESPACE foo_ts, ...)

This (presumably) forces all the indexes into the same tablespace,
so I don't find it to be a complete solution, just a wart.

We could get the same behavior with much less code if we redefined
LIKE to not try to copy the source indexes' tablespace(s). Then,
the user would set default_tablespace to get the effect of the
USING clause.

That would also make LIKE entirely free of tablespace permissions
hazards, so I'm starting to think more and more that that's really the
right definition.

regards, tom lane