Обсуждение: [PATCH] Add pg_get_tablespace_ddl() function to reconstruct CREATE TABLESPACE statement

Hi Manni,

Thanks for the patch!

On 29/10/2025 02:23, Manni Wood wrote:
> This patch creates a function pg_get_tablespace_ddl, designed to
> retrieve the full DDL statement for a tablespace. Users can obtain the
> DDL by providing the tablespace name, like so:
> 
>     SELECT pg_get_tablespace_ddl('regress_owner_tblsp');
>                                            pg_get_tablespace_ddl
>    
> ---------------------------------------------------------------------------------------------------
>      CREATE TABLESPACE regress_owner_tblsp OWNER regress_user LOCATION
> '' WITH (random_page_cost = 3);


Here my first comments regarding usability:

== quoted identifier ==

Tablespace names containing quoted identifiers cannot be parsed:

postgres=# CREATE TABLESPACE "My TS" LOCATION '/tmp/ts';
CREATE TABLESPACE
postgres=# SELECT pg_get_tablespace_ddl('"My TS"');
ERROR:  tablespace ""My TS"" does not exist

The following works, but I guess it shouldn't:

postgres=# SELECT pg_get_tablespace_ddl('My TS');
             pg_get_tablespace_ddl
-----------------------------------------------
 CREATE TABLESPACE "My TS" LOCATION '/tmp/ts';
(1 row)

The same applies for unicode characters:

postgres=# CREATE TABLESPACE "🐘" LOCATION '/tmp/ts';
CREATE TABLESPACE
postgres=# SELECT pg_get_tablespace_ddl('"🐘"');
ERROR:  tablespace ""🐘"" does not exist
postgres=# SELECT pg_get_tablespace_ddl('🐘');
           pg_get_tablespace_ddl
--------------------------------------------
 CREATE TABLESPACE "🐘" LOCATION '/tmp/ts';
(1 row)


== option precision ==

There is a precision loss in the options:

postgres=# CREATE TABLESPACE ts OWNER u1 LOCATION '/tmp/ts' WITH
(seq_page_cost = 1.12345678910, random_page_cost = 1.12345678910,
effective_io_concurrency = 17, maintenance_io_concurrency = 18);
CREATE TABLESPACE
postgres=# SELECT pg_get_tablespace_ddl('ts');

   pg_get_tablespace_ddl


---------------------------------------------------------------------------------------------------------------------------------------------
---------------------------------
 CREATE TABLESPACE ts OWNER u1 LOCATION '/tmp/ts' WITH (random_page_cost
= 1.12346, seq_page_cost = 1.12346, effective_io_concurrency = 17, m
aintenance_io_concurrency = 18);
(1 row)

\db shows it as in the CREATE TABLESPACE statement:

postgres=# \db+ ts

            List of tablespaces
 Name | Owner | Location | Access privileges |
                             Options
                         |  Size   | Description

------+-------+----------+-------------------+-----------------------------------------------------------------------------------------------
-------------------------+---------+-------------
 ts   | u1    | /tmp/ts  |                   |
{seq_page_cost=1.12345678910,random_page_cost=1.12345678910,effective_io_concurrency=17,mainte
nance_io_concurrency=18} | 0 bytes |
(1 row)


== permissions ==

Is it supposed to be visible to all users?

postgres=# CREATE USER u1;
CREATE ROLE
postgres=# CREATE TABLESPACE ts LOCATION '/tmp/ts';
CREATE TABLESPACE
postgres=# SET ROLE u1;
SET
postgres=> SELECT pg_get_tablespace_ddl('ts');
               pg_get_tablespace_ddl
----------------------------------------------------
 CREATE TABLESPACE ts OWNER jim LOCATION '/tmp/ts';
(1 row)

Note that \db does not allow it:

postgres=> SELECT CURRENT_USER;
 current_user
--------------
 u1
(1 row)

postgres=> \db+ ts
ERROR:  permission denied for tablespace ts


Best, Jim

On 04/11/2025 00:49, Manni Wood wrote:
> == quoted identifier ==
> 
>  I see that Postgres already has the SQL
> function has_tablespace_privilege that behaves the same way as this
> patch's pg_get_tablespace_ddl.

You're right. The source of my confusion is that I was approaching the
tablespace name as if it were a relation:

postgres=# CREATE TABLE "T"();
CREATE TABLE
postgres=# SELECT '"T"'::regclass::oid;
  oid
-------
 47766
(1 row)

postgres=# SELECT 'T'::regclass::oid;
ERROR:  relation "t" does not exist
LINE 1: SELECT 'T'::regclass::oid;

But I see that other functions behave similarly, e.g. pg_tablespace_size:

postgres=# SELECT pg_tablespace_size('My TS');
 pg_tablespace_size
--------------------
                  0
(1 row)

postgres=# SELECT pg_tablespace_size('"My TS"');
ERROR:  tablespace ""My TS"" does not exist
postgres=#

Sorry for the noise.

Do you think that an overload in pg_proc.dat with oid as parameter would
make sense here? e.g.

{ oid => '2322',
  descr => 'total disk space usage for the specified tablespace',
  proname => 'pg_tablespace_size', provolatile => 'v', prorettype => 'int8',
  proargtypes => 'oid', prosrc => 'pg_tablespace_size_oid' },
{ oid => '2323',
  descr => 'total disk space usage for the specified tablespace',
  proname => 'pg_tablespace_size', provolatile => 'v', prorettype => 'int8',
  proargtypes => 'name', prosrc => 'pg_tablespace_size_name' },


> 
> == option precision ==
> 
> Thanks for pointing this out.
> 
> I have attached a v2 of the patch that just uses the original text the
> user entered for the spcoptions.


Nice. It now shows the options without precision loss:

postgres=# CREATE TABLESPACE ts OWNER u1 LOCATION '/tmp/ts' WITH
(seq_page_cost = 1.12345678910, random_page_cost = 1.12345678910,
effective_io_concurrency = 17, maintenance_io_concurrency = 18);
CREATE TABLESPACE
postgres=# SELECT pg_get_tablespace_ddl('ts');

     pg_get_tablespace_ddl


-------------------------------------------------------------------------------------------------------------------------------------------
---------------------------------------
 CREATE TABLESPACE ts OWNER u1 LOCATION '/tmp/ts' WITH
(seq_page_cost=1.12345678910, random_page_cost=1.12345678910,
effective_io_concurrency=17, maintenance_io_concurrency=18);
(1 row)

postgres=# \db+ ts

            List of tablespaces
 Name | Owner | Location | Access privileges |
                             Options
                           |  Size   | Description

------+-------+----------+-------------------+---------------------------------------------------------------------------------------------
---------------------------+---------+-------------
 ts   | u1    | /tmp/ts  |                   |
{seq_page_cost=1.12345678910,random_page_cost=1.12345678910,effective_io_concurrency=17,maintenance_io_concurrency=18}
| 0 bytes |
(1 row)


> == permissions ==
> 
> I'm not sure what to think of this. psql's "\db+" does not let me show
> the tablespace.
> 

Right. I guess the difference here is that \db+ also shows the
tablespace's size, which requires the user to actually read it.

postgres=# CREATE TABLESPACE ts OWNER jim LOCATION '/tmp/ts';
CREATE TABLESPACE
postgres=# SELECT pg_tablespace_size('ts');
 pg_tablespace_size
--------------------
                  0
(1 row)

postgres=# SET ROLE u1;
SET
postgres=> SELECT pg_tablespace_size('ts');
ERROR:  permission denied for tablespace ts

Since pg_get_tablespace_ddl doesn't display size, I believe it's fine as-is.

Thanks.

Best, Jim

Hi Nishant

On 04/11/2025 11:37, Nishant Sharma wrote:
> Using name as parameter is more user friendly than OID.
> Because users usually do not know the oids. Constructing
> the DDL from the name appears better as it contains a name
> in it. So, no gain in having an OID version of
> pg_get_tablespace_ddl.

Would you also say that having a pg_tablespace_size(oid) has no benefit?

I took a look at similar functions, and the only pattern I could
identify is that all of them take an oid parameter.

pg_tablespace_size:       oid and name
pg_tablespace_location:   oid
has_tablespace_privilege: oid, name, and text
pg_tablespace_databases:  oid
...
pg_get_tablespace_ddl:    name

I'm definitely not opposed to having just a name parameter, but I
thought it would be worth mentioning.

Best, Jim

On 04/11/2025 15:19, Manni Wood wrote:
> I personally see no harm in additionally having an OID-based variant,
> seeing as it looks like a lot of functions do seem to take an OID. If I
> understand correctly, many functions take an OID, and Postgres users are
> supposed to have read the docs (https://www.postgresql.org/docs/current/
> datatype-oid.html <https://www.postgresql.org/docs/current/datatype-
> oid.html>) to know to cast names to OIDs. So, in terms of following
> established practice / patterns, an OID-based variant is defensible.

+1

That's the way I see it too. Of course it's always easier to use the
tablespace's name, but there might be cases where you only have the oid
-- in which case you'd need to do a JOIN with pg_tablespace to find the
name. It's just for convenience.

Best, Jim

On 2025-Nov-04, Jim Jones wrote:

> That's the way I see it too. Of course it's always easier to use the
> tablespace's name, but there might be cases where you only have the oid
> -- in which case you'd need to do a JOIN with pg_tablespace to find the
> name. It's just for convenience.

The other DDL-producing patches that are being posted, all depend on a
reg* type for their argument, which means they will work correctly with
either an OID or an object name.  Tablespaces are one of the few object
types for which no "regtablespace" exists, so I think it's fair to
require both forms.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
[…] indem ich in meinem Leben oft an euch gedacht, euch glücklich zu machen. Seyd es!
A menudo he pensado en vosotros, en haceros felices. ¡Sedlo, pues!
        Heiligenstädter Testament, L. v. Beethoven, 1802
        https://de.wikisource.org/wiki/Heiligenstädter_Testament

On 2025-Nov-05, Nishant Sharma wrote:

> My reasons why I thought only name form was sufficient:-
> 1. The use case that I had in my mind for this get DDL function was
> getting covered with name as its parameter. As we are creating DDL
> and name will be part of it. Hence using it as input to our function to
> create its DDL.

Accepting an OID as parameter lets the user call this function in a
query that returns a tablespace OID as parameter, without having to add
a join to pg_tablespace.  Not something you see very frequently, but I
can imagine GUI tool writers doing that.

> 4. The list of other tablespaces functions shared by Jim has two
> functions, pg_tablespace_location() & pg_tablespace_databases()
> that takes only oid as parameter and not name or text (maybe would
> have been better), why? I am not sure, maybe the use case at that
> time needed only an oid variant?

Lack of the other form of pg_tablespace_location has annoyed me on
occassion, but I don't think it's frequent enough to request it to be
added.  (I don't remember ever having a need to call
pg_tablespace_databases).

> +    /* Find tablespace directory path */
> +    datumlocation = DirectFunctionCall1(pg_tablespace_location, tspaceoid);
> +    path = text_to_cstring(DatumGetTextP(datumlocation));

It seems worth splitting pg_tablespace_location in two parts: one outer
shell that takes PG_FUNCTION_ARGS and returns text, and an inner
implementation function that takes a plain Oid and returns char *.  This
way, you can use the inner one here without the DirectFunctionCall1()
scaffolding, and avoid having to convert the laboriously constructed
text immediately back to a C string.

> +Datum
> +pg_get_tablespace_ddl_name(PG_FUNCTION_ARGS)
> +{
> +    PG_RETURN_TEXT_P(build_tablespace_ddl(get_tablespace_oid(NameStr(*(Name)PG_GETARG_NAME(0)),
> +                                                             false)));
> +}

This line is far too clever.  Better add a Name variable for
PG_GETARG_NAME(), an Oid variable for get_tablespace_oid(), and so on.
It'll be more code lines, but they will be ten times more readable.

> +Datum
> +pg_get_tablespace_ddl_oid(PG_FUNCTION_ARGS)
> +{
> +    PG_RETURN_TEXT_P(build_tablespace_ddl((Oid)PG_GETARG_OID(0)));
> +}

This one isn't _that_ bad -- still, our style is to have all the
PG_GETARG_foo() invocations together in an orderly fashion at the very
top of local variable declarations in pretty much all of our functions,
and I think that's a good habit to keep.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Digital and video cameras have this adjustment and film cameras don't for the
same reason dogs and cats lick themselves: because they can."   (Ken Rockwell)

On 2025-Nov-05, Nishant Sharma wrote:

> Thanks Álvaro for the review comments on v4!
> 
> PFA, v5 patch set. I have included all your review comments.

Great, thanks.

I think adding the get_tablespace_location_string function in
ruleutils.c makes little sense -- I would say it belongs in
src/backend/catalog/pg_tablespace.c.  Since this file happens not to
exist, you can create it.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Someone said that it is at least an order of magnitude more work to do
production software than a prototype. I think he is wrong by at least
an order of magnitude."                              (Brian Kernighan)

On 07/11/2025 02:27, Manni Wood wrote:
> Attached v7 fixes problems in tests due to whitespace.


Since get_tablespace_loc_string returns a palloc'd string, I guess you
could pfree it after the if block. The same applies for spcowner, since
you're calling GetUserNameFromId() with noerr = false.

For reference, see pg_get_indexdef_worker():

...
/*
 * If it has options, append "WITH (options)"
 */
str = flatten_reloptions(indexrelid);
if (str)
{
  appendStringInfo(&buf, " WITH (%s)", str);
  pfree(str);
}
...


Thanks

Best, Jim

On 08/11/2025 00:38, Manni Wood wrote:
> Alas, the build https://commitfest.postgresql.org/patch/6175/ <https://
> commitfest.postgresql.org/patch/6175/> now fails, and I cannot reproduce
> on my machine. Obviously there will be a v9...

You forgot the declaration for build_tablespace_ddl_string[1]:

ruleutils.c:13755:1: warning: no previous prototype for
‘build_tablespace_ddl_string’ [-Wmissing-prototypes]
13755 | build_tablespace_ddl_string(const Oid tspaceoid)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Best, Jim

1 - https://cirrus-ci.com/task/4855404196265984?logs=build#L1911

On 08/11/2025 15:05, Manni Wood wrote:
> Apologies for the noise. Just wanted to confirm that I did a fresh clone
> of github.com/postgres/postgres <http://github.com/postgres/postgres> on
> my linux machine, applied both v9 patches to master, ran 
> "configure", "make world", "make check-world", and everything passed.


Perhaps a missing entry at src/backend/catalog/meson.build for
pg_tablespace.c? You probably don't see the error in your environment
because you're not building with meson.

Best, Jim

On 2025-Nov-08, Manni Wood wrote:

> However, I see that this
> https://cirrus-ci.com/task/6437176629526528?logs=clone#L214 checks out a
> specific commit on a specific branch:
> 
> "Checked out db131410131cb6a60f074213b0e7aaaa15d72f87 on cf/6175 branch."
> 
> My clone of postgres (which is presumably shallow?) does not show that
> branch, not even with "git branch -r".

Yeah, these branches used by CI are made up on the spot and aren't
propagated out of that repository.  You can add a "remote" to your
existing clone,

git remote add cfbot https://github.com/postgresql-cfbot/postgresql.git

and then you can see the branches it creates and `git switch` to them.
Do mind that they are ephemeral, and for this thread they only contain
the patches you submitted yourself; really, this is only useful if
you're going to review other people's patches, and even then it might
still be better to use the patches from the mailing list (which is where
cfbot itself grabs them from anyway).

You should definitely be reviewing other people's patches, though, not
just because it's valuable for the community as a whole to have an
additional pair of eyes on them, but also because they can teach you
many things.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Oh, great altar of passive entertainment, bestow upon me thy discordant images
at such speed as to render linear thought impossible" (Calvin a la TV)

On 2025-Nov-10, Nishant Sharma wrote:

> PFA, v10 patch set.

I propose the following changes for 0001, in patch hunk ordering.

1. pg_tablespace_location was introduced in 2011 (commit 16d8e594acd9),
so claim copyright starting at that point.

2. readlink(2) claims, at least in my system, to need <unistd.h>.  Add
that.

3. get_tablespace_loc_string() is such an ugly name.  Why not
get_tablespace_location()?

3. The initialization of sourcepath and targetpath are mostly pointless
(see below), so I'd leave it out.

3a. (Also, it's not clear to me that initializing to "{ '\0' }" is a
great idea.  I understand that the C standard says that an
initialization to {0} zeroes the whole struct, but if you try to pass
some other char value, it actually fills everything else with zeroes
rather than the other char value.  So hiding the 0 byte as a \0 char is
misleading.)

3b. Also, if you had zeroed targetpath at initialization time, there
would no longer be a need to print a zero byte after calling readlink(),
so you could have removed the "targetpath[rllen] = '\0';" line.
However as I said above, I'm not a fan of unnecessary initialization.

4. Using StringInfo in this function is pointless.  You use that when
you're going to do a bunch of string manipulation ops, appending more
data after the first, or using sprintf() formatted strings and so on.
But here you return just one or two possible strings with no
construction involved.  Might as well use standard pstrdup() as needed,
which keeps the code simple.

5. Single-statement blocks need no braces.

6. ereport() used to require an extra set of parenthesis, but no more.
Remove those.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Tiene valor aquel que admite que es un cobarde" (Fernandel)

On 2025-Nov-12, Manni Wood wrote:

> Thanks, Álvaro, for your continued help with this.
> 
> I have attached v11 patches that use all of the fixes from your
> review.patch.txt.

OK, thanks, I pushed 0001 now.

I think you could claim that some routines currently in
src/backend/commands/tablespace.c logically belong in the new file, but
unless you want to take on the task of moving a lot of other routines
under commands/ to their respective catalog/ file, then I think it's
more or less fine as is.

To be clear, I do not intend to do anything with your 0002 patch [for
now].  I'm going to let Andrew take these DDL-producing functions in his
hands.  Here I'm just posting your 0002 again, to make the cfbot happy.

Thanks

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)

Hi Manni,

I just reviewed and tested the patch, just got a few small comments:

> On Nov 13, 2025, at 00:15, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Nov-12, Manni Wood wrote:
>
>> Thanks, Álvaro, for your continued help with this.
>>
>> I have attached v11 patches that use all of the fixes from your
>> review.patch.txt.
>
> OK, thanks, I pushed 0001 now.
>
> I think you could claim that some routines currently in
> src/backend/commands/tablespace.c logically belong in the new file, but
> unless you want to take on the task of moving a lot of other routines
> under commands/ to their respective catalog/ file, then I think it's
> more or less fine as is.
>
> To be clear, I do not intend to do anything with your 0002 patch [for
> now].  I'm going to let Andrew take these DDL-producing functions in his
> hands.  Here I'm just posting your 0002 again, to make the cfbot happy.
>
> Thanks
>
> --
> Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
> "Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)
> <v12-0002-Adds-pg_get_tablespace_ddl-function.patch>


1.
```
+                 errmsg("tablespace with oid %d does not exist",
```

Existing code all use “%u” to format oid. You may search for “oid %” to see that.

2.
```
+            /* Add the options in WITH clause */
+            appendStringInfoString(&buf, TextDatumGetCString(optdatums[i]));
+            appendStringInfoString(&buf, ", “);
```

This two statements can be combined into one:

appendStringInfoString(&buf,  “%s, “,  TextDatumGetCString(optdatums[i]));

3
```
+        if (strncmp(PG_TBLSPC_DIR_SLASH, path, strlen(PG_TBLSPC_DIR_SLASH)) == 0)
+            appendStringInfoString(&buf, " LOCATION ''");
+        else
+            appendStringInfo(&buf, " LOCATION '%s'", path);
+    }
```

Instead of hardcoding single-quotes, we can use quote_literal_cstr().

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Hi Manni,

I just reviewed v13 again, and still got a couple of comments:

> On Nov 18, 2025, at 22:15, Manni Wood <manni.wood@enterprisedb.com> wrote:
>
>
> <v13-0001-Adds-pg_get_tablespace_ddl-function.patch>

1. Do we need to perform some privilege check? I just did a test:
```
evantest=> \c
You are now connected to database "evantest" as user "evan".
evantest=> select pg_get_tablespace_ddl('pg_default');
           pg_get_tablespace_ddl
-------------------------------------------
 CREATE TABLESPACE pg_default OWNER chaol;
(1 row)
```

Where “evan” is a new user without grant any persuasion to it, but it can view the system default tablespace’s DDL. I
don’tthink that’s expected. 

2.
```
+        optarray = DatumGetArrayTypeP(datum);
+
+        deconstruct_array_builtin(optarray, TEXTOID,
+                                  &optdatums, NULL, &optcount);
+
+        Assert(optcount);
+
+        /* Start WITH clause */
+        appendStringInfoString(&buf, " WITH (");
+
+        for (i = 0; i < (optcount - 1); i++)    /* Skipping last option */
+        {
+            /* Add the options in WITH clause */
+            appendStringInfo(&buf, "%s, ", TextDatumGetCString(optdatums[i]));
+        }
+
+        /* Adding the last remaining option */
+        appendStringInfoString(&buf, TextDatumGetCString(optdatums[i]));
```

This block of code is a duplicate of get_reloptions() defined in ruleutils.c, and get_reloptions() performs more
checks.So I think build_tablespace_ddl_string() should be defined in ruleutils.c and reuse the existing helper
function.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Hi Chao

On 19/11/2025 04:13, Chao Li wrote:
> 1. Do we need to perform some privilege check? I just did a test:
> ```
> evantest=> \c
> You are now connected to database "evantest" as user "evan".
> evantest=> select pg_get_tablespace_ddl('pg_default');
>            pg_get_tablespace_ddl
> -------------------------------------------
>  CREATE TABLESPACE pg_default OWNER chaol;
> (1 row)
> ```
> 
> Where “evan” is a new user without grant any persuasion to it, but it can view the system default tablespace’s DDL. I
don’tthink that’s expected.
 

It is expected. \db behaves similarly:


CREATE TABLESPACE ts LOCATION '/tmp/ts';
CREATE TABLESPACE

postgres=# CREATE USER foo;
CREATE ROLE

postgres=# SET ROLE foo;
SET

postgres=> \db ts
   List of tablespaces
 Name | Owner | Location
------+-------+----------
 ts   | jim   | /tmp/ts
(1 row)


IIUC the user foo is just reading the catalog entry of the new
tablespace, which is fine. Of course, accessing the tablespace itself is
not allowed. See \db+ (calculates the tablespace's size)


postgres=> \db+ ts
ERROR:  permission denied for tablespace ts

Best, Jim