| From: | Manni Wood <manni(dot)wood(at)enterprisedb(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: [PATCH] Add pg_get_tablespace_ddl() function to reconstruct CREATE TABLESPACE statement |
| Date: | 2025-11-18 14:15:03 |
| Message-ID: | CAKWEB6qUq+-QBSTsO05ZigumNQEvZfqCafKv-S7eDiutOK8raA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Nov 13, 2025 at 12:07 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> 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(at)kurilemu(dot)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/
>
>
>
>
>
Hello, Chao Li!
Thank you for the improvements to my patch, helping it follow more of
Postgres's coding conventions. Much appreciated.
I have attached v13 of the patch. Now that Álvaro has merged the contents
of the 0001 patch, I assume this v13 patch can be 0001 and not 0002.
--
-- Manni Wood EDB: https://www.enterprisedb.com
| Attachment | Content-Type | Size |
|---|---|---|
| v13-0001-Adds-pg_get_tablespace_ddl-function.patch | text/x-patch | 15.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Heikki Linnakangas | 2025-11-18 14:15:36 | Re: GUC thread-safety approaches |
| Previous Message | David Geier | 2025-11-18 14:06:54 | Re: Performance issues with parallelism and LIMIT |