Hi Manni, I just reviewed v13 again, and still got a couple of comments:
> On Nov 18, 2025, at 22:15, Manni Wood <[email protected]> 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’t think 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/
