Re: [PATCH] Add pg_get_tablespace_ddl() function to reconstruct CREATE TABLESPACE statement - Mailing list pgsql-hackers

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/