Re: Tablespace patch review - Mailing list pgsql-patches

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> OK, I have reviewed the patch.  I think Tom is doing the same, but I
> want to report the things I found.

I just came up for air after about two solid days of working on this
patch ... had not seen your message before committing it.  The good
news is that I think I did see all the stuff you found.

> What facility is there for moving objects between tablespaces?

None, as yet.

> Seems we should be consistent in having WIN32 defs or not.

Probably.  I removed #ifdefs whereever possible --- there are just a few
left in tablespace.c and dbcommands.c now.  I was contemplating
replacing HAVE_SYMLINKS with a HAVE_TABLESPACES flag, but with the
occurrences isolated to one file I'm not sure it's worth the trouble.

> Your code in tablespc.c calls realpath().  Do all OS's have that?

It doesn't anymore --- I was concerned about the portability question
too.  The only point of that code AFAICS was to prevent creation of
two pg_tablespace entries pointing at the same directory.  I felt that
the better way to handle this was to write a PG_VERSION file in the
tablespace directory during CREATE TABLESPACE.  A subsequent CREATE
TABLESPACE on the same directory will fail because the directory isn't
empty anymore.  And the version file might come in handy someday...

> Does all object creation code put a lock in the tablespace row to
> prevent DROP TABLESPACE from removing a tablespace in use?

I hacked up some logic to deal with this, based on taking out an
ExclusiveLock on pg_tablespace when adding a per-database subdirectory
to a tablespace directory or doing DROP TABLESPACE.  It works but it'd
be nice to reduce the strength of the lock ...

> There is interesting code that checks to see of the objects existing in
> a tablespace are about to be dropped by the transaction.

s/interesting/doesn't work/ ... I didn't commit this stuff.  Nor the
smgr changes either; those were far from ready for prime time.

> Are we ripping out our initlocation code at the same time?

Not done yet, but it's dead and should be removed as soon as a
decent respect for the deceased permits ;-)

> Where do we need to add mention of tablespaces in the main
> non-reference-page docs?  Clearly at least in the section on managing
> disk space.

Yeah.  The patch as committed covers the reference pages, but we
desperately need a higher-level discussion of tablespaces for the
administrator's guide.

            regards, tom lane