Hi Manni, I just reviewed and tested the patch, just got a few small comments:
> On Nov 13, 2025, at 00:15, Álvaro Herrera <[email protected]> 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/
